Skip to content

Commit

Permalink
Updated Storage account (#169)
Browse files Browse the repository at this point in the history
  • Loading branch information
abner-dou authored Mar 18, 2022
1 parent 942f90d commit 9cffeab
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 49 deletions.
8 changes: 7 additions & 1 deletion internal/services/storage/storage_account_data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -155,6 +156,11 @@ func storageAccountDataSource() *schema.Resource {
},

"tags": tags.SchemaDataSource(),

"https_traffic_only_enabled": {
Type: schema.TypeBool,
Computed: true,
},
},
}
}
Expand Down Expand Up @@ -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 {
Expand Down
28 changes: 27 additions & 1 deletion internal/services/storage/storage_account_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -213,6 +214,12 @@ func storageAccount() *schema.Resource {
},

"tags": tags.Schema(),

"enable_https_traffic_only": {
Type: pluginsdk.TypeBool,
Optional: true,
Default: true,
},
},
}
}
Expand All @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
Expand Down
97 changes: 51 additions & 46 deletions internal/services/storage/storage_account_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{}

Expand All @@ -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
})
}

Expand Down Expand Up @@ -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{}
Expand Down Expand Up @@ -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(`
Expand All @@ -216,7 +199,7 @@ resource "azurestack_storage_account" "import" {
}
`, template)
}

*/
func (r StorageAccountResource) premium(data acceptance.TestData) string {
return fmt.Sprintf(`
provider "azurestack" {
Expand All @@ -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 {}
Expand All @@ -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"
Expand All @@ -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 {}
Expand All @@ -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 {}
Expand All @@ -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"
Expand Down
39 changes: 39 additions & 0 deletions internal/services/storage/validate/storage_account_tags.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
3 changes: 3 additions & 0 deletions website/docs/d/storage_account.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 4 additions & 1 deletion website/docs/r/storage_account.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

---

Expand Down

0 comments on commit 9cffeab

Please sign in to comment.