From 9cffeaba6bc10e901a8cd16fdced0e3ab2b7784a Mon Sep 17 00:00:00 2001 From: Abner Garcia <84406577+abner-dou@users.noreply.github.com> Date: Thu, 17 Mar 2022 18:35:43 -0700 Subject: [PATCH] Updated Storage account (#169) --- .../storage/storage_account_data_source.go | 8 +- .../storage/storage_account_resource.go | 28 +++++- .../storage/storage_account_resource_test.go | 97 ++++++++++--------- .../storage/validate/storage_account_tags.go | 39 ++++++++ website/docs/d/storage_account.html.markdown | 3 + website/docs/r/storage_account.html.markdown | 5 +- 6 files changed, 131 insertions(+), 49 deletions(-) create mode 100644 internal/services/storage/validate/storage_account_tags.go diff --git a/internal/services/storage/storage_account_data_source.go b/internal/services/storage/storage_account_data_source.go index 2f07bd4ac..32bc32e9f 100644 --- a/internal/services/storage/storage_account_data_source.go +++ b/internal/services/storage/storage_account_data_source.go @@ -12,6 +12,7 @@ import ( "github.com/hashicorp/go-azure-helpers/resourcemanager/commonschema" "github.com/hashicorp/go-azure-helpers/resourcemanager/location" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" + "github.com/hashicorp/terraform-provider-azurestack/internal/az/tags" "github.com/hashicorp/terraform-provider-azurestack/internal/clients" "github.com/hashicorp/terraform-provider-azurestack/internal/services/storage/parse" @@ -155,6 +156,11 @@ func storageAccountDataSource() *schema.Resource { }, "tags": tags.SchemaDataSource(), + + "https_traffic_only_enabled": { + Type: schema.TypeBool, + Computed: true, + }, }, } } @@ -221,7 +227,7 @@ func storageAccountDataSourceRead(d *schema.ResourceData, meta interface{}) erro // Computed d.Set("primary_location", props.PrimaryLocation) d.Set("secondary_location", props.SecondaryLocation) - + d.Set("https_traffic_only_enabled", props.EnableHTTPSTrafficOnly) if encryption := props.Encryption; encryption != nil { if services := encryption.Services; services != nil { if blob := services.Blob; blob != nil { diff --git a/internal/services/storage/storage_account_resource.go b/internal/services/storage/storage_account_resource.go index 11d603b35..64219fa45 100644 --- a/internal/services/storage/storage_account_resource.go +++ b/internal/services/storage/storage_account_resource.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/go-azure-helpers/resourcemanager/location" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" + "github.com/hashicorp/terraform-provider-azurestack/internal/az/tags" "github.com/hashicorp/terraform-provider-azurestack/internal/clients" "github.com/hashicorp/terraform-provider-azurestack/internal/services/storage/migration" @@ -213,6 +214,12 @@ func storageAccount() *schema.Resource { }, "tags": tags.Schema(), + + "enable_https_traffic_only": { + Type: pluginsdk.TypeBool, + Optional: true, + Default: true, + }, }, } } @@ -227,6 +234,7 @@ func storageAccountCreate(d *schema.ResourceData, meta interface{}) error { accountKind := d.Get("account_kind").(string) location := d.Get("location").(string) + enableHTTPSTrafficOnly := d.Get("enable_https_traffic_only").(bool) accountTier := d.Get("account_tier").(string) replicationType := d.Get("account_replication_type").(string) @@ -249,7 +257,9 @@ func storageAccountCreate(d *schema.ResourceData, meta interface{}) error { Kind: storage.Kind(accountKind), // If any paramers are specified withouth the right values this will fail - AccountPropertiesCreateParameters: &storage.AccountPropertiesCreateParameters{}, + AccountPropertiesCreateParameters: &storage.AccountPropertiesCreateParameters{ + EnableHTTPSTrafficOnly: &enableHTTPSTrafficOnly, + }, } if _, ok := d.GetOk("custom_domain"); ok { @@ -392,6 +402,20 @@ func storageAccountUpdate(d *schema.ResourceData, meta interface{}) error { } } + if d.HasChange("enable_https_traffic_only") { + enableHTTPSTrafficOnly := d.Get("enable_https_traffic_only").(bool) + + opts := storage.AccountUpdateParameters{ + AccountPropertiesUpdateParameters: &storage.AccountPropertiesUpdateParameters{ + EnableHTTPSTrafficOnly: &enableHTTPSTrafficOnly, + }, + } + + if _, err := client.Update(ctx, id.ResourceGroup, id.Name, opts); err != nil { + return fmt.Errorf("updating Azure Storage Account enable_https_traffic_only %q: %+v", id.Name, err) + } + } + d.Partial(false) return nil } @@ -448,6 +472,8 @@ func storageAccountRead(d *schema.ResourceData, meta interface{}) error { d.Set("account_encryption_source", string(encryption.KeySource)) } + d.Set("enable_https_traffic_only", props.EnableHTTPSTrafficOnly) + // Computed d.Set("primary_location", props.PrimaryLocation) d.Set("secondary_location", props.SecondaryLocation) diff --git a/internal/services/storage/storage_account_resource_test.go b/internal/services/storage/storage_account_resource_test.go index 5a7e77597..9f3c86f6d 100644 --- a/internal/services/storage/storage_account_resource_test.go +++ b/internal/services/storage/storage_account_resource_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/hashicorp/go-azure-helpers/lang/pointer" + "github.com/hashicorp/terraform-provider-azurestack/internal/clients" "github.com/hashicorp/terraform-provider-azurestack/internal/services/storage/parse" "github.com/hashicorp/terraform-provider-azurestack/internal/tf/acceptance" @@ -47,6 +48,11 @@ func TestAccStorageAccount_basic(t *testing.T) { } func TestAccStorageAccount_requiresImport(t *testing.T) { + t.Skip("test skipped, please check comments inside this test") + /*This test is skipped due to a bug in the Azure autorest client, when the Create request function is used, + instead of using a POST request use sa PUT which causes the update of the previous resource instead of throwing + an 'Already exists' error. + */ data := acceptance.BuildTestData(t, "azurestack_storage_account", "test") r := StorageAccountResource{} @@ -57,7 +63,7 @@ func TestAccStorageAccount_requiresImport(t *testing.T) { check.That(data.ResourceName).ExistsInAzure(r), ), }, - data.RequiresImportErrorStep(r.requiresImport), + // data.RequiresImportErrorStep(r.requiresImport), // TODO: uncomment this until the bug gets resolved }) } @@ -106,30 +112,6 @@ func TestAccStorageAccount_blobConnectionString(t *testing.T) { }) } -// TODO does stack support this now? -/* -func TestAccStorageAccount_blobStorageWithUpdate(t *testing.T) { - data := acceptance.BuildTestData(t, "azurestack_storage_account", "test") - r := StorageAccountResource{} - - data.ResourceTest(t, r, []acceptance.TestStep{ - { - Config: r.blobStorage(data), - Check: acceptance.ComposeTestCheckFunc( - check.That(data.ResourceName).ExistsInAzure(r), - check.That(data.ResourceName).Key("account_kind").HasValue("BlobStorage"), - ), - }, - data.ImportStep(), - { - Config: r.blobStorageUpdate(data), - Check: acceptance.ComposeTestCheckFunc( - check.That(data.ResourceName).ExistsInAzure(r), - ), - }, - }) -}*/ - func TestAccStorageAccount_NonStandardCasing(t *testing.T) { data := acceptance.BuildTestData(t, "azurestack_storage_account", "test") r := StorageAccountResource{} @@ -202,6 +184,7 @@ resource "azurestack_storage_account" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomString) } +/* func (r StorageAccountResource) requiresImport(data acceptance.TestData) string { template := r.basic(data) return fmt.Sprintf(` @@ -216,7 +199,7 @@ resource "azurestack_storage_account" "import" { } `, template) } - +*/ func (r StorageAccountResource) premium(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurestack" { @@ -243,8 +226,7 @@ resource "azurestack_storage_account" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomString) } -/* -func (r StorageAccountResource) blobStorage(data acceptance.TestData) string { +func (r StorageAccountResource) nonStandardCasing(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurestack" { features {} @@ -256,13 +238,11 @@ resource "azurestack_resource_group" "test" { } resource "azurestack_storage_account" "test" { - name = "unlikely23exst2acct%s" - resource_group_name = azurestack_resource_group.test.name - + name = "unlikely23exst2acct%s" + resource_group_name = azurestack_resource_group.test.name location = azurestack_resource_group.test.location - account_kind = "BlobStorage" - account_tier = "Standard" - account_replication_type = "LRS" + account_tier = "standard" + account_replication_type = "lrs" tags = { environment = "production" @@ -271,7 +251,30 @@ resource "azurestack_storage_account" "test" { `, data.RandomInteger, data.Locations.Primary, data.RandomString) } -func (r StorageAccountResource) blobStorageUpdate(data acceptance.TestData) string { +func TestAccStorageAccount_enableHttpsTrafficOnly(t *testing.T) { + data := acceptance.BuildTestData(t, "azurestack_storage_account", "test") + r := StorageAccountResource{} + + data.ResourceTest(t, r, []acceptance.TestStep{ + { + Config: r.enableHttpsTrafficOnly(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("enable_https_traffic_only").HasValue("true"), + ), + }, + data.ImportStep(), + { + Config: r.enableHttpsTrafficOnlyDisabled(data), + Check: acceptance.ComposeTestCheckFunc( + check.That(data.ResourceName).ExistsInAzure(r), + check.That(data.ResourceName).Key("enable_https_traffic_only").HasValue("false"), + ), + }, + }) +} + +func (r StorageAccountResource) enableHttpsTrafficOnly(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurestack" { features {} @@ -286,19 +289,19 @@ resource "azurestack_storage_account" "test" { name = "unlikely23exst2acct%s" resource_group_name = azurestack_resource_group.test.name - location = azurestack_resource_group.test.location - account_kind = "BlobStorage" - account_tier = "Standard" - account_replication_type = "LRS" + location = azurestack_resource_group.test.location + account_tier = "Standard" + account_replication_type = "LRS" + enable_https_traffic_only = true tags = { environment = "production" } } `, data.RandomInteger, data.Locations.Primary, data.RandomString) -}*/ +} -func (r StorageAccountResource) nonStandardCasing(data acceptance.TestData) string { +func (r StorageAccountResource) enableHttpsTrafficOnlyDisabled(data acceptance.TestData) string { return fmt.Sprintf(` provider "azurestack" { features {} @@ -310,11 +313,13 @@ resource "azurestack_resource_group" "test" { } resource "azurestack_storage_account" "test" { - name = "unlikely23exst2acct%s" - resource_group_name = azurestack_resource_group.test.name - location = azurestack_resource_group.test.location - account_tier = "standard" - account_replication_type = "lrs" + name = "unlikely23exst2acct%s" + resource_group_name = azurestack_resource_group.test.name + + location = azurestack_resource_group.test.location + account_tier = "Standard" + account_replication_type = "LRS" + enable_https_traffic_only = false tags = { environment = "production" diff --git a/internal/services/storage/validate/storage_account_tags.go b/internal/services/storage/validate/storage_account_tags.go new file mode 100644 index 000000000..d74bbd70d --- /dev/null +++ b/internal/services/storage/validate/storage_account_tags.go @@ -0,0 +1,39 @@ +package validate + +import ( + "fmt" +) + +func StorageAccountTags(v interface{}, _ string) (warnings []string, errors []error) { + tagsMap := v.(map[string]interface{}) + + if len(tagsMap) > 50 { + errors = append(errors, fmt.Errorf("a maximum of 50 tags can be applied to storage account ARM resource")) + } + + for k, v := range tagsMap { + if len(k) > 128 { + errors = append(errors, fmt.Errorf("the maximum length for a tag key is 128 characters: %q is %d characters", k, len(k))) + } + + value, err := TagValueToString(v) + if err != nil { + errors = append(errors, err) + } else if len(value) > 256 { + errors = append(errors, fmt.Errorf("the maximum length for a tag value is 256 characters: the value for %q is %d characters", k, len(value))) + } + } + + return warnings, errors +} + +func TagValueToString(v interface{}) (string, error) { + switch value := v.(type) { + case string: + return value, nil + case int: + return fmt.Sprintf("%d", value), nil + default: + return "", fmt.Errorf("unknown tag type %T in tag value", value) + } +} diff --git a/website/docs/d/storage_account.html.markdown b/website/docs/d/storage_account.html.markdown index e8a6e9b05..b62e70590 100644 --- a/website/docs/d/storage_account.html.markdown +++ b/website/docs/d/storage_account.html.markdown @@ -48,6 +48,9 @@ output "storage_account_tier" { * `tags` - A mapping of tags to assigned to the resource. +* `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. + * `primary_location` - The primary location of the Storage Account. * `secondary_location` - The secondary location of the Storage Account. diff --git a/website/docs/r/storage_account.html.markdown b/website/docs/r/storage_account.html.markdown index f55b84bae..ec62baed8 100644 --- a/website/docs/r/storage_account.html.markdown +++ b/website/docs/r/storage_account.html.markdown @@ -57,7 +57,10 @@ The following arguments are supported: * `custom_domain` - (Optional) A `custom_domain` block as documented below. -* `tags` - (Optional) A mapping of tags to assign to the resource. +* `tags` - (Optional) A mapping of tags to assign to the resource.\ + +* `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`. ---