Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated Storage account #169

Merged
merged 4 commits into from
Mar 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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