From a9adbac105c92dba583db95381f71e58ab97e1b4 Mon Sep 17 00:00:00 2001 From: neil-yechenwei Date: Wed, 12 Aug 2020 13:10:32 +0800 Subject: [PATCH 1/7] - filter and from Azure US Government --- .../services/storage/client/client.go | 28 ++++----- .../storage/resource_arm_storage_account.go | 59 +++++++++++++------ website/docs/r/storage_account.html.markdown | 4 ++ 3 files changed, 59 insertions(+), 32 deletions(-) diff --git a/azurerm/internal/services/storage/client/client.go b/azurerm/internal/services/storage/client/client.go index 25a2e40f2e70..94b3f377caff 100644 --- a/azurerm/internal/services/storage/client/client.go +++ b/azurerm/internal/services/storage/client/client.go @@ -29,7 +29,7 @@ type Client struct { StorageTargetsClient *storagecache.StorageTargetsClient SubscriptionId string - environment az.Environment + Environment az.Environment storageAdAuth *autorest.Authorizer } @@ -62,7 +62,7 @@ func NewClient(options *common.ClientOptions) *Client { CachesClient: &cachesClient, SubscriptionId: options.SubscriptionId, StorageTargetsClient: &storageTargetsClient, - environment: options.Environment, + Environment: options.Environment, } if options.StorageUseAzureAD { @@ -74,7 +74,7 @@ func NewClient(options *common.ClientOptions) *Client { func (client Client) AccountsDataPlaneClient(ctx context.Context, account accountDetails) (*accounts.Client, error) { if client.storageAdAuth != nil { - accountsClient := accounts.NewWithEnvironment(client.environment) + accountsClient := accounts.NewWithEnvironment(client.Environment) accountsClient.Client.Authorizer = *client.storageAdAuth return &accountsClient, nil } @@ -89,14 +89,14 @@ func (client Client) AccountsDataPlaneClient(ctx context.Context, account accoun return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - accountsClient := accounts.NewWithEnvironment(client.environment) + accountsClient := accounts.NewWithEnvironment(client.Environment) accountsClient.Client.Authorizer = storageAuth return &accountsClient, nil } func (client Client) BlobsClient(ctx context.Context, account accountDetails) (*blobs.Client, error) { if client.storageAdAuth != nil { - blobsClient := blobs.NewWithEnvironment(client.environment) + blobsClient := blobs.NewWithEnvironment(client.Environment) blobsClient.Client.Authorizer = *client.storageAdAuth return &blobsClient, nil } @@ -111,14 +111,14 @@ func (client Client) BlobsClient(ctx context.Context, account accountDetails) (* return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - blobsClient := blobs.NewWithEnvironment(client.environment) + blobsClient := blobs.NewWithEnvironment(client.Environment) blobsClient.Client.Authorizer = storageAuth return &blobsClient, nil } func (client Client) ContainersClient(ctx context.Context, account accountDetails) (*containers.Client, error) { if client.storageAdAuth != nil { - containersClient := containers.NewWithEnvironment(client.environment) + containersClient := containers.NewWithEnvironment(client.Environment) containersClient.Client.Authorizer = *client.storageAdAuth return &containersClient, nil } @@ -133,7 +133,7 @@ func (client Client) ContainersClient(ctx context.Context, account accountDetail return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - containersClient := containers.NewWithEnvironment(client.environment) + containersClient := containers.NewWithEnvironment(client.Environment) containersClient.Client.Authorizer = storageAuth return &containersClient, nil } @@ -151,7 +151,7 @@ func (client Client) FileShareDirectoriesClient(ctx context.Context, account acc return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - directoriesClient := directories.NewWithEnvironment(client.environment) + directoriesClient := directories.NewWithEnvironment(client.Environment) directoriesClient.Client.Authorizer = storageAuth return &directoriesClient, nil } @@ -169,14 +169,14 @@ func (client Client) FileSharesClient(ctx context.Context, account accountDetail return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - sharesClient := shares.NewWithEnvironment(client.environment) + sharesClient := shares.NewWithEnvironment(client.Environment) sharesClient.Client.Authorizer = storageAuth return &sharesClient, nil } func (client Client) QueuesClient(ctx context.Context, account accountDetails) (*queues.Client, error) { if client.storageAdAuth != nil { - queueAuth := queues.NewWithEnvironment(client.environment) + queueAuth := queues.NewWithEnvironment(client.Environment) queueAuth.Client.Authorizer = *client.storageAdAuth return &queueAuth, nil } @@ -191,7 +191,7 @@ func (client Client) QueuesClient(ctx context.Context, account accountDetails) ( return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - queuesClient := queues.NewWithEnvironment(client.environment) + queuesClient := queues.NewWithEnvironment(client.Environment) queuesClient.Client.Authorizer = storageAuth return &queuesClient, nil } @@ -209,7 +209,7 @@ func (client Client) TableEntityClient(ctx context.Context, account accountDetai return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - entitiesClient := entities.NewWithEnvironment(client.environment) + entitiesClient := entities.NewWithEnvironment(client.Environment) entitiesClient.Client.Authorizer = storageAuth return &entitiesClient, nil } @@ -227,7 +227,7 @@ func (client Client) TablesClient(ctx context.Context, account accountDetails) ( return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - tablesClient := tables.NewWithEnvironment(client.environment) + tablesClient := tables.NewWithEnvironment(client.Environment) tablesClient.Client.Authorizer = storageAuth return &tablesClient, nil } diff --git a/azurerm/internal/services/storage/resource_arm_storage_account.go b/azurerm/internal/services/storage/resource_arm_storage_account.go index 7af3277ec026..59d25a1bb74d 100644 --- a/azurerm/internal/services/storage/resource_arm_storage_account.go +++ b/azurerm/internal/services/storage/resource_arm_storage_account.go @@ -10,6 +10,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-04-01/storage" azautorest "github.com/Azure/go-autorest/autorest" + autorestAzure "github.com/Azure/go-autorest/autorest/azure" "github.com/hashicorp/go-azure-helpers/response" "github.com/hashicorp/go-getter/helper/url" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" @@ -604,6 +605,7 @@ func validateAzureRMStorageAccountTags(v interface{}, _ string) (warnings []stri } func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) error { + env := meta.(*clients.Client).Storage.Environment client := meta.(*clients.Client).Storage.AccountsClient ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d) defer cancel() @@ -648,13 +650,19 @@ func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) e Kind: storage.Kind(accountKind), AccountPropertiesCreateParameters: &storage.AccountPropertiesCreateParameters{ EnableHTTPSTrafficOnly: &enableHTTPSTrafficOnly, - MinimumTLSVersion: storage.MinimumTLSVersion(minimumTLSVersion), NetworkRuleSet: expandStorageAccountNetworkRules(d), IsHnsEnabled: &isHnsEnabled, - AllowBlobPublicAccess: &allowBlobPublicAccess, }, } + // For US Government Cloud, don't specify "allow_blob_public_access" and "min_tls_version" in request body. + // https://github.com/terraform-providers/terraform-provider-azurerm/issues/7812 + // https://github.com/terraform-providers/terraform-provider-azurerm/issues/8083 + if env.Name != autorestAzure.USGovernmentCloud.Name { + parameters.AccountPropertiesCreateParameters.AllowBlobPublicAccess = &allowBlobPublicAccess + parameters.AccountPropertiesCreateParameters.MinimumTLSVersion = storage.MinimumTLSVersion(minimumTLSVersion) + } + if _, ok := d.GetOk("identity"); ok { storageAccountIdentity := expandAzureRmStorageAccountIdentity(d) parameters.Identity = storageAccountIdentity @@ -784,6 +792,7 @@ func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) e } func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) error { + env := meta.(*clients.Client).Storage.Environment client := meta.(*clients.Client).Storage.AccountsClient ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d) defer cancel() @@ -888,28 +897,36 @@ func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) e if d.HasChange("min_tls_version") { minimumTLSVersion := d.Get("min_tls_version").(string) - opts := storage.AccountUpdateParameters{ - AccountPropertiesUpdateParameters: &storage.AccountPropertiesUpdateParameters{ - MinimumTLSVersion: storage.MinimumTLSVersion(minimumTLSVersion), - }, - } + // For US Government Cloud, don't specify "min_tls_version" in request body. + // https://github.com/terraform-providers/terraform-provider-azurerm/issues/8083 + if env.Name != autorestAzure.USGovernmentCloud.Name { + opts := storage.AccountUpdateParameters{ + AccountPropertiesUpdateParameters: &storage.AccountPropertiesUpdateParameters{ + MinimumTLSVersion: storage.MinimumTLSVersion(minimumTLSVersion), + }, + } - if _, err := client.Update(ctx, resourceGroupName, storageAccountName, opts); err != nil { - return fmt.Errorf("Error updating Azure Storage Account min_tls_version %q: %+v", storageAccountName, err) + if _, err := client.Update(ctx, resourceGroupName, storageAccountName, opts); err != nil { + return fmt.Errorf("Error updating Azure Storage Account min_tls_version %q: %+v", storageAccountName, err) + } } } if d.HasChange("allow_blob_public_access") { allowBlobPublicAccess := d.Get("allow_blob_public_access").(bool) - opts := storage.AccountUpdateParameters{ - AccountPropertiesUpdateParameters: &storage.AccountPropertiesUpdateParameters{ - AllowBlobPublicAccess: &allowBlobPublicAccess, - }, - } + // For US Government Cloud, don't specify "allow_blob_public_access" in request body. + // https://github.com/terraform-providers/terraform-provider-azurerm/issues/7812 + if env.Name != autorestAzure.USGovernmentCloud.Name { + 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_blob_public_access %q: %+v", storageAccountName, err) + if _, err := client.Update(ctx, resourceGroupName, storageAccountName, opts); err != nil { + return fmt.Errorf("Error updating Azure Storage Account allow_blob_public_access %q: %+v", storageAccountName, err) + } } } @@ -1005,6 +1022,7 @@ func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) e } func resourceArmStorageAccountRead(d *schema.ResourceData, meta interface{}) error { + env := meta.(*clients.Client).Storage.Environment client := meta.(*clients.Client).Storage.AccountsClient endpointSuffix := meta.(*clients.Client).Account.Environment.StorageEndpointSuffix ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) @@ -1066,9 +1084,14 @@ 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("min_tls_version", string(props.MinimumTLSVersion)) d.Set("is_hns_enabled", props.IsHnsEnabled) - d.Set("allow_blob_public_access", props.AllowBlobPublicAccess) + // For US Government Cloud, don't specify "allow_blob_public_access" and "min_tls_version" in request body. + // https://github.com/terraform-providers/terraform-provider-azurerm/issues/7812 + // https://github.com/terraform-providers/terraform-provider-azurerm/issues/8083 + if env.Name != autorestAzure.USGovernmentCloud.Name { + d.Set("allow_blob_public_access", props.AllowBlobPublicAccess) + d.Set("min_tls_version", string(props.MinimumTLSVersion)) + } if customDomain := props.CustomDomain; customDomain != nil { if err := d.Set("custom_domain", flattenStorageAccountCustomDomain(customDomain)); err != nil { diff --git a/website/docs/r/storage_account.html.markdown b/website/docs/r/storage_account.html.markdown index 8995d98e6d14..ffc0b3f03e3c 100644 --- a/website/docs/r/storage_account.html.markdown +++ b/website/docs/r/storage_account.html.markdown @@ -99,8 +99,12 @@ The following arguments are supported: * `min_tls_version` - (Optional) The minimum supported TLS version for the storage account. Possible values are `TLS1_0`, `TLS1_1`, and `TLS1_2`. Defaults to `TLS1_0` for new storage accounts. +-> **NOTE:** At this time `min_tls_version` is not supported in US Government. + * `allow_blob_public_access` - Allow or disallow public access to all blobs or containers in the storage account. Defaults to `false`. +-> **NOTE:** At this time `allow_blob_public_access` is not supported in US Government. + * `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 ab3e4efd10bf8d0bdfaa76eff096736346c54c17 Mon Sep 17 00:00:00 2001 From: neil-yechenwei Date: Wed, 12 Aug 2020 15:31:30 +0800 Subject: [PATCH 2/7] Update code --- .../storage/resource_arm_storage_account.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/azurerm/internal/services/storage/resource_arm_storage_account.go b/azurerm/internal/services/storage/resource_arm_storage_account.go index 59d25a1bb74d..2ba8003f56bf 100644 --- a/azurerm/internal/services/storage/resource_arm_storage_account.go +++ b/azurerm/internal/services/storage/resource_arm_storage_account.go @@ -661,6 +661,12 @@ func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) e if env.Name != autorestAzure.USGovernmentCloud.Name { parameters.AccountPropertiesCreateParameters.AllowBlobPublicAccess = &allowBlobPublicAccess parameters.AccountPropertiesCreateParameters.MinimumTLSVersion = storage.MinimumTLSVersion(minimumTLSVersion) + } else { + // As removing default value for "allow_blob_public_access" and "min_tls_version" is breaking change, so default value has to be kept. + // So when "allow_blob_public_access" and "min_tls_version" are default value, they would be set as empty value in request body for provisioning successfully. + 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`, env.Name) + } } if _, ok := d.GetOk("identity"); ok { @@ -909,6 +915,12 @@ 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 min_tls_version %q: %+v", storageAccountName, err) } + } else { + // As removing default value for "min_tls_version" is breaking change, so default value has to be kept. + // So when "min_tls_version" is default value, it would be set as empty value in request body for provisioning successfully. + if minimumTLSVersion != string(storage.TLS10) { + return fmt.Errorf(`"min_tls_version" is not supported for a Storage Account located in %q`, env.Name) + } } } @@ -927,6 +939,12 @@ 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_blob_public_access %q: %+v", storageAccountName, err) } + } else { + // As removing default value for "allow_blob_public_access" is breaking change, so default value has to be kept. + // So when "allow_blob_public_access" is default value, it would be set as empty value in request body for provisioning successfully. + if allowBlobPublicAccess { + return fmt.Errorf(`"allow_blob_public_access" is not supported for a Storage Account located in %q`, env.Name) + } } } From 66f2569d178ed37841af18c5a4d0f0ff439b0f2a Mon Sep 17 00:00:00 2001 From: neil-yechenwei Date: Wed, 12 Aug 2020 17:46:44 +0800 Subject: [PATCH 3/7] Update code --- .../storage/resource_arm_storage_account.go | 34 ++++++++----------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/azurerm/internal/services/storage/resource_arm_storage_account.go b/azurerm/internal/services/storage/resource_arm_storage_account.go index 2ba8003f56bf..07ba81bb3526 100644 --- a/azurerm/internal/services/storage/resource_arm_storage_account.go +++ b/azurerm/internal/services/storage/resource_arm_storage_account.go @@ -658,15 +658,13 @@ func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) e // For US Government Cloud, don't specify "allow_blob_public_access" and "min_tls_version" in request body. // https://github.com/terraform-providers/terraform-provider-azurerm/issues/7812 // https://github.com/terraform-providers/terraform-provider-azurerm/issues/8083 - if env.Name != autorestAzure.USGovernmentCloud.Name { - parameters.AccountPropertiesCreateParameters.AllowBlobPublicAccess = &allowBlobPublicAccess - parameters.AccountPropertiesCreateParameters.MinimumTLSVersion = storage.MinimumTLSVersion(minimumTLSVersion) - } else { - // As removing default value for "allow_blob_public_access" and "min_tls_version" is breaking change, so default value has to be kept. - // So when "allow_blob_public_access" and "min_tls_version" are default value, they would be set as empty value in request body for provisioning successfully. + if env.Name == autorestAzure.USGovernmentCloud.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`, env.Name) } + } else { + parameters.AccountPropertiesCreateParameters.AllowBlobPublicAccess = &allowBlobPublicAccess + parameters.AccountPropertiesCreateParameters.MinimumTLSVersion = storage.MinimumTLSVersion(minimumTLSVersion) } if _, ok := d.GetOk("identity"); ok { @@ -905,7 +903,11 @@ func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) e // For US Government Cloud, don't specify "min_tls_version" in request body. // https://github.com/terraform-providers/terraform-provider-azurerm/issues/8083 - if env.Name != autorestAzure.USGovernmentCloud.Name { + if env.Name == autorestAzure.USGovernmentCloud.Name { + if minimumTLSVersion != string(storage.TLS10) { + return fmt.Errorf(`"min_tls_version" is not supported for a Storage Account located in %q`, env.Name) + } + } else { opts := storage.AccountUpdateParameters{ AccountPropertiesUpdateParameters: &storage.AccountPropertiesUpdateParameters{ MinimumTLSVersion: storage.MinimumTLSVersion(minimumTLSVersion), @@ -915,12 +917,6 @@ 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 min_tls_version %q: %+v", storageAccountName, err) } - } else { - // As removing default value for "min_tls_version" is breaking change, so default value has to be kept. - // So when "min_tls_version" is default value, it would be set as empty value in request body for provisioning successfully. - if minimumTLSVersion != string(storage.TLS10) { - return fmt.Errorf(`"min_tls_version" is not supported for a Storage Account located in %q`, env.Name) - } } } @@ -929,7 +925,11 @@ func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) e // For US Government Cloud, don't specify "allow_blob_public_access" in request body. // https://github.com/terraform-providers/terraform-provider-azurerm/issues/7812 - if env.Name != autorestAzure.USGovernmentCloud.Name { + if env.Name == autorestAzure.USGovernmentCloud.Name { + if allowBlobPublicAccess { + return fmt.Errorf(`"allow_blob_public_access" is not supported for a Storage Account located in %q`, env.Name) + } + } else { opts := storage.AccountUpdateParameters{ AccountPropertiesUpdateParameters: &storage.AccountPropertiesUpdateParameters{ AllowBlobPublicAccess: &allowBlobPublicAccess, @@ -939,12 +939,6 @@ 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_blob_public_access %q: %+v", storageAccountName, err) } - } else { - // As removing default value for "allow_blob_public_access" is breaking change, so default value has to be kept. - // So when "allow_blob_public_access" is default value, it would be set as empty value in request body for provisioning successfully. - if allowBlobPublicAccess { - return fmt.Errorf(`"allow_blob_public_access" is not supported for a Storage Account located in %q`, env.Name) - } } } From e9386bc271af6fef89152429d0f0f57da7c9f4ee Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Wed, 12 Aug 2020 15:46:27 -0700 Subject: [PATCH 4/7] Revert changes to client and update env checks --- .../services/storage/client/client.go | 28 +++++++++---------- .../storage/resource_arm_storage_account.go | 19 ++++++------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/azurerm/internal/services/storage/client/client.go b/azurerm/internal/services/storage/client/client.go index 94b3f377caff..25a2e40f2e70 100644 --- a/azurerm/internal/services/storage/client/client.go +++ b/azurerm/internal/services/storage/client/client.go @@ -29,7 +29,7 @@ type Client struct { StorageTargetsClient *storagecache.StorageTargetsClient SubscriptionId string - Environment az.Environment + environment az.Environment storageAdAuth *autorest.Authorizer } @@ -62,7 +62,7 @@ func NewClient(options *common.ClientOptions) *Client { CachesClient: &cachesClient, SubscriptionId: options.SubscriptionId, StorageTargetsClient: &storageTargetsClient, - Environment: options.Environment, + environment: options.Environment, } if options.StorageUseAzureAD { @@ -74,7 +74,7 @@ func NewClient(options *common.ClientOptions) *Client { func (client Client) AccountsDataPlaneClient(ctx context.Context, account accountDetails) (*accounts.Client, error) { if client.storageAdAuth != nil { - accountsClient := accounts.NewWithEnvironment(client.Environment) + accountsClient := accounts.NewWithEnvironment(client.environment) accountsClient.Client.Authorizer = *client.storageAdAuth return &accountsClient, nil } @@ -89,14 +89,14 @@ func (client Client) AccountsDataPlaneClient(ctx context.Context, account accoun return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - accountsClient := accounts.NewWithEnvironment(client.Environment) + accountsClient := accounts.NewWithEnvironment(client.environment) accountsClient.Client.Authorizer = storageAuth return &accountsClient, nil } func (client Client) BlobsClient(ctx context.Context, account accountDetails) (*blobs.Client, error) { if client.storageAdAuth != nil { - blobsClient := blobs.NewWithEnvironment(client.Environment) + blobsClient := blobs.NewWithEnvironment(client.environment) blobsClient.Client.Authorizer = *client.storageAdAuth return &blobsClient, nil } @@ -111,14 +111,14 @@ func (client Client) BlobsClient(ctx context.Context, account accountDetails) (* return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - blobsClient := blobs.NewWithEnvironment(client.Environment) + blobsClient := blobs.NewWithEnvironment(client.environment) blobsClient.Client.Authorizer = storageAuth return &blobsClient, nil } func (client Client) ContainersClient(ctx context.Context, account accountDetails) (*containers.Client, error) { if client.storageAdAuth != nil { - containersClient := containers.NewWithEnvironment(client.Environment) + containersClient := containers.NewWithEnvironment(client.environment) containersClient.Client.Authorizer = *client.storageAdAuth return &containersClient, nil } @@ -133,7 +133,7 @@ func (client Client) ContainersClient(ctx context.Context, account accountDetail return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - containersClient := containers.NewWithEnvironment(client.Environment) + containersClient := containers.NewWithEnvironment(client.environment) containersClient.Client.Authorizer = storageAuth return &containersClient, nil } @@ -151,7 +151,7 @@ func (client Client) FileShareDirectoriesClient(ctx context.Context, account acc return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - directoriesClient := directories.NewWithEnvironment(client.Environment) + directoriesClient := directories.NewWithEnvironment(client.environment) directoriesClient.Client.Authorizer = storageAuth return &directoriesClient, nil } @@ -169,14 +169,14 @@ func (client Client) FileSharesClient(ctx context.Context, account accountDetail return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - sharesClient := shares.NewWithEnvironment(client.Environment) + sharesClient := shares.NewWithEnvironment(client.environment) sharesClient.Client.Authorizer = storageAuth return &sharesClient, nil } func (client Client) QueuesClient(ctx context.Context, account accountDetails) (*queues.Client, error) { if client.storageAdAuth != nil { - queueAuth := queues.NewWithEnvironment(client.Environment) + queueAuth := queues.NewWithEnvironment(client.environment) queueAuth.Client.Authorizer = *client.storageAdAuth return &queueAuth, nil } @@ -191,7 +191,7 @@ func (client Client) QueuesClient(ctx context.Context, account accountDetails) ( return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - queuesClient := queues.NewWithEnvironment(client.Environment) + queuesClient := queues.NewWithEnvironment(client.environment) queuesClient.Client.Authorizer = storageAuth return &queuesClient, nil } @@ -209,7 +209,7 @@ func (client Client) TableEntityClient(ctx context.Context, account accountDetai return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - entitiesClient := entities.NewWithEnvironment(client.Environment) + entitiesClient := entities.NewWithEnvironment(client.environment) entitiesClient.Client.Authorizer = storageAuth return &entitiesClient, nil } @@ -227,7 +227,7 @@ func (client Client) TablesClient(ctx context.Context, account accountDetails) ( return nil, fmt.Errorf("Error building Authorizer: %+v", err) } - tablesClient := tables.NewWithEnvironment(client.Environment) + tablesClient := tables.NewWithEnvironment(client.environment) tablesClient.Client.Authorizer = storageAuth return &tablesClient, nil } diff --git a/azurerm/internal/services/storage/resource_arm_storage_account.go b/azurerm/internal/services/storage/resource_arm_storage_account.go index 07ba81bb3526..2bfbda03bf2c 100644 --- a/azurerm/internal/services/storage/resource_arm_storage_account.go +++ b/azurerm/internal/services/storage/resource_arm_storage_account.go @@ -605,7 +605,7 @@ func validateAzureRMStorageAccountTags(v interface{}, _ string) (warnings []stri } func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) error { - env := meta.(*clients.Client).Storage.Environment + envName := meta.(*clients.Client).Account.Environment.Name client := meta.(*clients.Client).Storage.AccountsClient ctx, cancel := timeouts.ForCreate(meta.(*clients.Client).StopContext, d) defer cancel() @@ -658,9 +658,9 @@ func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) e // For US Government Cloud, don't specify "allow_blob_public_access" and "min_tls_version" in request body. // https://github.com/terraform-providers/terraform-provider-azurerm/issues/7812 // https://github.com/terraform-providers/terraform-provider-azurerm/issues/8083 - if env.Name == autorestAzure.USGovernmentCloud.Name { + if envName == autorestAzure.USGovernmentCloud.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`, env.Name) + return fmt.Errorf(`"allow_blob_public_access" and "min_tls_version" are not supported for a Storage Account located in %q`, envName) } } else { parameters.AccountPropertiesCreateParameters.AllowBlobPublicAccess = &allowBlobPublicAccess @@ -796,7 +796,7 @@ func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) e } func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) error { - env := meta.(*clients.Client).Storage.Environment + envName := meta.(*clients.Client).Account.Environment.Name client := meta.(*clients.Client).Storage.AccountsClient ctx, cancel := timeouts.ForUpdate(meta.(*clients.Client).StopContext, d) defer cancel() @@ -903,9 +903,9 @@ func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) e // For US Government Cloud, don't specify "min_tls_version" in request body. // https://github.com/terraform-providers/terraform-provider-azurerm/issues/8083 - if env.Name == autorestAzure.USGovernmentCloud.Name { + if envName == autorestAzure.USGovernmentCloud.Name { if minimumTLSVersion != string(storage.TLS10) { - return fmt.Errorf(`"min_tls_version" is not supported for a Storage Account located in %q`, env.Name) + return fmt.Errorf(`"min_tls_version" is not supported for a Storage Account located in %q`, envName) } } else { opts := storage.AccountUpdateParameters{ @@ -925,9 +925,9 @@ func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) e // For US Government Cloud, don't specify "allow_blob_public_access" in request body. // https://github.com/terraform-providers/terraform-provider-azurerm/issues/7812 - if env.Name == autorestAzure.USGovernmentCloud.Name { + if envName == autorestAzure.USGovernmentCloud.Name { if allowBlobPublicAccess { - return fmt.Errorf(`"allow_blob_public_access" is not supported for a Storage Account located in %q`, env.Name) + return fmt.Errorf(`"allow_blob_public_access" is not supported for a Storage Account located in %q`, envName) } } else { opts := storage.AccountUpdateParameters{ @@ -1034,7 +1034,6 @@ func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) e } func resourceArmStorageAccountRead(d *schema.ResourceData, meta interface{}) error { - env := meta.(*clients.Client).Storage.Environment client := meta.(*clients.Client).Storage.AccountsClient endpointSuffix := meta.(*clients.Client).Account.Environment.StorageEndpointSuffix ctx, cancel := timeouts.ForRead(meta.(*clients.Client).StopContext, d) @@ -1100,7 +1099,7 @@ func resourceArmStorageAccountRead(d *schema.ResourceData, meta interface{}) err // For US Government Cloud, don't specify "allow_blob_public_access" and "min_tls_version" in request body. // https://github.com/terraform-providers/terraform-provider-azurerm/issues/7812 // https://github.com/terraform-providers/terraform-provider-azurerm/issues/8083 - if env.Name != autorestAzure.USGovernmentCloud.Name { + if meta.(*clients.Client).Account.Environment.Name != autorestAzure.USGovernmentCloud.Name { d.Set("allow_blob_public_access", props.AllowBlobPublicAccess) d.Set("min_tls_version", string(props.MinimumTLSVersion)) } From ca99469544e337129acf703eed339a0aae37f166 Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Wed, 12 Aug 2020 16:24:01 -0700 Subject: [PATCH 5/7] Update test cases --- .../resource_arm_storage_account_test.go | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) 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 04b8f7f93f59..25e101030f7d 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 @@ -6,6 +6,7 @@ import ( "regexp" "testing" + autorestAzure "github.com/Azure/go-autorest/autorest/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/storage" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" @@ -281,6 +282,10 @@ func TestAccAzureRMStorageAccount_minTLSVersion(t *testing.T) { } func TestAccAzureRMStorageAccount_allowBlobPublicAccess(t *testing.T) { + if ok := environmentUSGovernment(); ok { + t.Skip("Skipping `allowBlobPublicAccess` test only valid in Public Cloud") + } + data := acceptance.BuildTestData(t, "azurerm_storage_account", "test") resource.ParallelTest(t, resource.TestCase{ @@ -315,6 +320,29 @@ func TestAccAzureRMStorageAccount_allowBlobPublicAccess(t *testing.T) { }) } +func TestAccAzureRMStorageAccount_allowBlobPublicAccessUSGov(t *testing.T) { + if ok := environmentUSGovernment(); !ok { + t.Skip("Skipping `allowBlobPublicAccessUSGov` test only valid in US Government Cloud") + } + + 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_basic(data), + ExpectError: regexp.MustCompile(`allow_blob_public_access" and "min_tls_version" are not supported for a Storage Account located in AzureUSGovernmentCloud`), + Check: resource.ComposeTestCheckFunc( + testCheckAzureRMStorageAccountExists(data.ResourceName), + ), + }, + }, + }) +} + func TestAccAzureRMStorageAccount_isHnsEnabled(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_account", "test") @@ -810,6 +838,14 @@ func testCheckAzureRMStorageAccountExists(resourceName string) resource.TestChec } } +func environmentUSGovernment() bool { + if acceptance.AzureProvider.Meta().(*clients.Client).Account.Environment.Name == autorestAzure.USGovernmentCloud.Name { + return true + } + + return false +} + func testCheckAzureRMStorageAccountDisappears(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { // Ensure resource group exists in API From 13c95d1e4c03b67f2ef3337fc8ffed0a00ecf418 Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Wed, 12 Aug 2020 17:00:57 -0700 Subject: [PATCH 6/7] Revert gov env test case --- .../resource_arm_storage_account_test.go | 36 ------------------- 1 file changed, 36 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 25e101030f7d..04b8f7f93f59 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 @@ -6,7 +6,6 @@ import ( "regexp" "testing" - autorestAzure "github.com/Azure/go-autorest/autorest/azure" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/internal/services/storage" "github.com/terraform-providers/terraform-provider-azurerm/azurerm/helpers/validate" @@ -282,10 +281,6 @@ func TestAccAzureRMStorageAccount_minTLSVersion(t *testing.T) { } func TestAccAzureRMStorageAccount_allowBlobPublicAccess(t *testing.T) { - if ok := environmentUSGovernment(); ok { - t.Skip("Skipping `allowBlobPublicAccess` test only valid in Public Cloud") - } - data := acceptance.BuildTestData(t, "azurerm_storage_account", "test") resource.ParallelTest(t, resource.TestCase{ @@ -320,29 +315,6 @@ func TestAccAzureRMStorageAccount_allowBlobPublicAccess(t *testing.T) { }) } -func TestAccAzureRMStorageAccount_allowBlobPublicAccessUSGov(t *testing.T) { - if ok := environmentUSGovernment(); !ok { - t.Skip("Skipping `allowBlobPublicAccessUSGov` test only valid in US Government Cloud") - } - - 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_basic(data), - ExpectError: regexp.MustCompile(`allow_blob_public_access" and "min_tls_version" are not supported for a Storage Account located in AzureUSGovernmentCloud`), - Check: resource.ComposeTestCheckFunc( - testCheckAzureRMStorageAccountExists(data.ResourceName), - ), - }, - }, - }) -} - func TestAccAzureRMStorageAccount_isHnsEnabled(t *testing.T) { data := acceptance.BuildTestData(t, "azurerm_storage_account", "test") @@ -838,14 +810,6 @@ func testCheckAzureRMStorageAccountExists(resourceName string) resource.TestChec } } -func environmentUSGovernment() bool { - if acceptance.AzureProvider.Meta().(*clients.Client).Account.Environment.Name == autorestAzure.USGovernmentCloud.Name { - return true - } - - return false -} - func testCheckAzureRMStorageAccountDisappears(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { // Ensure resource group exists in API From b27f2af04cf90f1177cdd4a1882863ccc5b21df8 Mon Sep 17 00:00:00 2001 From: Jeffrey Cline <20408400+WodansSon@users.noreply.github.com> Date: Wed, 12 Aug 2020 21:05:01 -0700 Subject: [PATCH 7/7] Fix set logic --- .../services/storage/resource_arm_storage_account.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/azurerm/internal/services/storage/resource_arm_storage_account.go b/azurerm/internal/services/storage/resource_arm_storage_account.go index 2bfbda03bf2c..2fbb319d386a 100644 --- a/azurerm/internal/services/storage/resource_arm_storage_account.go +++ b/azurerm/internal/services/storage/resource_arm_storage_account.go @@ -1096,11 +1096,13 @@ func resourceArmStorageAccountRead(d *schema.ResourceData, meta interface{}) err d.Set("access_tier", props.AccessTier) d.Set("enable_https_traffic_only", props.EnableHTTPSTrafficOnly) d.Set("is_hns_enabled", props.IsHnsEnabled) - // For US Government Cloud, don't specify "allow_blob_public_access" and "min_tls_version" in request body. + d.Set("allow_blob_public_access", props.AllowBlobPublicAccess) + // For US Government Cloud, "min_tls_version" is not returned from Azure so always persist the default values for "min_tls_version". // https://github.com/terraform-providers/terraform-provider-azurerm/issues/7812 // https://github.com/terraform-providers/terraform-provider-azurerm/issues/8083 - if meta.(*clients.Client).Account.Environment.Name != autorestAzure.USGovernmentCloud.Name { - d.Set("allow_blob_public_access", props.AllowBlobPublicAccess) + if meta.(*clients.Client).Account.Environment.Name == autorestAzure.USGovernmentCloud.Name { + d.Set("min_tls_version", string(storage.TLS10)) + } else { d.Set("min_tls_version", string(props.MinimumTLSVersion)) }