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

Storage Account: Add identity property #1323

Merged
merged 11 commits into from
Jun 14, 2018
79 changes: 78 additions & 1 deletion azurerm/resource_arm_storage_account.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,33 @@ func resourceArmStorageAccount() *schema.Resource {
Sensitive: true,
},

"identity": {
Type: schema.TypeList,
Optional: true,
Computed: true,
MaxItems: 1,
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"type": {
Type: schema.TypeString,
Required: true,
DiffSuppressFunc: ignoreCaseDiffSuppressFunc,
ValidateFunc: validation.StringInSlice([]string{
"SystemAssigned",
}, true),
},
"principal_id": {
Type: schema.TypeString,
Computed: true,
},
"tenant_id": {
Type: schema.TypeString,
Computed: true,
},
},
},
},

"tags": tagsSchema(),
},
}
Expand Down Expand Up @@ -308,6 +335,11 @@ func resourceArmStorageAccountCreate(d *schema.ResourceData, meta interface{}) e
},
}

if _, ok := d.GetOk("identity"); ok {
storageAccountIdentity := expandAzureRmStorageAccountIdentity(d)
parameters.Identity = storageAccountIdentity
}

if _, ok := d.GetOk("custom_domain"); ok {
parameters.CustomDomain = expandStorageAccountCustomDomain(d)
}
Expand Down Expand Up @@ -495,6 +527,18 @@ func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) e
d.SetPartial("enable_https_traffic_only")
}

if d.HasChange("identity") {
storageAccountIdentity := expandAzureRmStorageAccountIdentity(d)

opts := storage.AccountUpdateParameters{
Identity: storageAccountIdentity,
}
_, err := client.Update(ctx, resourceGroupName, storageAccountName, opts)
if err != nil {
return fmt.Errorf("Error updating Azure Storage Account identity %q: %+v", storageAccountName, err)
}
}

if d.HasChange("network_rules") {
networkRules := expandStorageAccountNetworkRules(d)

Expand All @@ -512,7 +556,7 @@ func resourceArmStorageAccountUpdate(d *schema.ResourceData, meta interface{}) e
}

d.Partial(false)
return nil
return resourceArmStorageAccountRead(d, meta)
}

func resourceArmStorageAccountRead(d *schema.ResourceData, meta interface{}) error {
Expand Down Expand Up @@ -637,6 +681,11 @@ func resourceArmStorageAccountRead(d *schema.ResourceData, meta interface{}) err
d.Set("primary_access_key", accessKeys[0].Value)
d.Set("secondary_access_key", accessKeys[1].Value)

identity := flattenAzureRmStorageAccountIdentity(resp.Identity)
if err := d.Set("identity", identity); err != nil {
return err
}

flattenAndSetTags(d, resp.Tags)

return nil
Expand Down Expand Up @@ -813,3 +862,31 @@ func validateArmStorageAccountType(v interface{}, k string) (ws []string, es []e
es = append(es, fmt.Errorf("Invalid storage account type %q", input))
return
}

func expandAzureRmStorageAccountIdentity(d *schema.ResourceData) *storage.Identity {
identities := d.Get("identity").([]interface{})
identity := identities[0].(map[string]interface{})
identityType := identity["type"].(string)
return &storage.Identity{
Type: &identityType,
}
}

func flattenAzureRmStorageAccountIdentity(identity *storage.Identity) []interface{} {
if identity == nil {
return make([]interface{}, 0)
}

result := make(map[string]interface{})
if identity.Type != nil {
result["type"] = *identity.Type
}
if identity.PrincipalID != nil {
result["principal_id"] = *identity.PrincipalID
}
if identity.TenantID != nil {
result["tenant_id"] = *identity.TenantID
}

return []interface{}{result}
}
100 changes: 90 additions & 10 deletions azurerm/resource_arm_storage_account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package azurerm
import (
"fmt"
"net/http"
"regexp"
"testing"

"github.com/hashicorp/terraform/helper/acctest"
Expand Down Expand Up @@ -345,6 +346,69 @@ func TestAccAzureRMStorageAccount_NonStandardCasing(t *testing.T) {
})
}

func TestAccAzureRMStorageAccount_enableIdentity(t *testing.T) {
resourceName := "azurerm_storage_account.testsa"

ri := acctest.RandInt()
rs := acctest.RandString(4)
config := testAccAzureRMStorageAccount_identity(ri, rs, testLocation())

uuidMatch := regexp.MustCompile("^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-4[a-fA-F0-9]{3}-[8|9|aA|bB][a-fA-F0-9]{3}-[a-fA-F0-9]{12}$")

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMStorageAccountDestroy,
Steps: []resource.TestStep{
{
Config: config,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMStorageAccountExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "identity.0.type", "SystemAssigned"),
resource.TestMatchResourceAttr(resourceName, "identity.0.principal_id", uuidMatch),
resource.TestMatchResourceAttr(resourceName, "identity.0.tenant_id", uuidMatch),
),
},
},
})
}

func TestAccAzureRMStorageAccount_updateResourceByEnablingIdentity(t *testing.T) {
resourceName := "azurerm_storage_account.testsa"

ri := acctest.RandInt()
rs := acctest.RandString(4)

basicResourceNoManagedIdentity := testAccAzureRMStorageAccount_basic(ri, rs, testLocation())
managedIdentityEnabled := testAccAzureRMStorageAccount_identity(ri, rs, testLocation())

uuidMatch := regexp.MustCompile("^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-4[a-fA-F0-9]{3}-[8|9|aA|bB][a-fA-F0-9]{3}-[a-fA-F0-9]{12}$")

resource.Test(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testCheckAzureRMStorageAccountDestroy,
Steps: []resource.TestStep{
{
Config: basicResourceNoManagedIdentity,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMStorageAccountExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "identity.#", "0"),
),
},
{
Config: managedIdentityEnabled,
Check: resource.ComposeTestCheckFunc(
testCheckAzureRMStorageAccountExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "identity.0.type", "SystemAssigned"),
resource.TestMatchResourceAttr(resourceName, "identity.0.principal_id", uuidMatch),
resource.TestMatchResourceAttr(resourceName, "identity.0.tenant_id", uuidMatch),
),
},
},
})
}

func TestAccAzureRMStorageAccount_networkRules(t *testing.T) {
resourceName := "azurerm_storage_account.testsa"
ri := acctest.RandInt()
Expand Down Expand Up @@ -805,32 +869,54 @@ resource "azurerm_storage_account" "testsa" {
`, rInt, location, rString)
}

func testAccAzureRMStorageAccount_networkRules(rInt int, rString string, location string) string {
func testAccAzureRMStorageAccount_identity(rInt int, rString string, location string) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "testrg" {
name = "testAccAzureRMSA-%d"
location = "%s"
}

resource "azurerm_storage_account" "testsa" {
name = "unlikely23exst2acct%s"
resource_group_name = "${azurerm_resource_group.testrg.name}"

location = "${azurerm_resource_group.testrg.location}"
account_tier = "Standard"
account_replication_type = "LRS"

identity {
type = "SystemAssigned"
}

tags {
environment = "production"
}
}
`, rInt, location, rString)
}

func testAccAzureRMStorageAccount_networkRules(rInt int, rString string, location string) string {
return fmt.Sprintf(`
resource "azurerm_resource_group" "testrg" {
name = "testAccAzureRMSA-%d"
location = "%s"
}
resource "azurerm_virtual_network" "test" {
name = "acctestvirtnet%d"
address_space = ["10.0.0.0/16"]
location = "${azurerm_resource_group.testrg.location}"
resource_group_name = "${azurerm_resource_group.testrg.name}"
}

resource "azurerm_subnet" "test" {
name = "acctestsubnet%d"
resource_group_name = "${azurerm_resource_group.testrg.name}"
virtual_network_name = "${azurerm_virtual_network.test.name}"
address_prefix = "10.0.2.0/24"
service_endpoints = ["Microsoft.Storage"]
}

resource "azurerm_storage_account" "testsa" {
name = "unlikely23exst2acct%s"
resource_group_name = "${azurerm_resource_group.testrg.name}"

location = "${azurerm_resource_group.testrg.location}"
account_tier = "Standard"
account_replication_type = "LRS"
Expand All @@ -839,7 +925,6 @@ resource "azurerm_storage_account" "testsa" {
ip_rules = ["127.0.0.1"]
virtual_network_subnet_ids = ["${azurerm_subnet.test.id}"]
}

tags {
environment = "production"
}
Expand All @@ -853,26 +938,22 @@ resource "azurerm_resource_group" "testrg" {
name = "testAccAzureRMSA-%d"
location = "%s"
}

resource "azurerm_virtual_network" "test" {
name = "acctestvirtnet%d"
address_space = ["10.0.0.0/16"]
location = "${azurerm_resource_group.testrg.location}"
resource_group_name = "${azurerm_resource_group.testrg.name}"
}

resource "azurerm_subnet" "test" {
name = "acctestsubnet%d"
resource_group_name = "${azurerm_resource_group.testrg.name}"
virtual_network_name = "${azurerm_virtual_network.test.name}"
address_prefix = "10.0.2.0/24"
service_endpoints = ["Microsoft.Storage"]
}

resource "azurerm_storage_account" "testsa" {
name = "unlikely23exst2acct%s"
resource_group_name = "${azurerm_resource_group.testrg.name}"

location = "${azurerm_resource_group.testrg.location}"
account_tier = "Standard"
account_replication_type = "LRS"
Expand All @@ -881,7 +962,6 @@ resource "azurerm_storage_account" "testsa" {
ip_rules = ["127.0.0.1", "127.0.0.2"]
bypass = ["Logging", "Metrics"]
}

tags {
environment = "production"
}
Expand Down
25 changes: 23 additions & 2 deletions website/docs/r/storage_account.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ The following arguments are supported:
* `location` - (Required) Specifies the supported Azure location where the
resource exists. Changing this forces a new resource to be created.

* `account_kind` - (Optional) Defines the Kind of account. Valid options are `Storage`,
`StorageV2` and `BlobStorage`. Changing this forces a new resource to be created.
* `account_kind` - (Optional) Defines the Kind of account. Valid options are `Storage`,
`StorageV2` and `BlobStorage`. Changing this forces a new resource to be created.
Defaults to `Storage`.

* `account_tier` - (Required) Defines the Tier to use for this storage account. Valid options are `Standard` and `Premium`. Changing this forces a new resource to be created
Expand All @@ -112,6 +112,8 @@ The following arguments are supported:

* `tags` - (Optional) A mapping of tags to assign to the resource.

* `identity` - (Optional) A Managed Service Identity block as defined below.

---

* `custom_domain` supports the following:
Expand All @@ -130,6 +132,14 @@ any combination of `Logging`, `Metrics`, `AzureServices`, or `None`.

~> **Note:** [More information on Validation is available here](https://docs.microsoft.com/en-gb/azure/storage/blobs/storage-custom-domain-name)

---

`identity` supports the following:

* `type` - (Required) Specifies the identity type of the Storage Account. At this time the only allowed value is `SystemAssigned`.

~> The assigned `principal_id` and `tenant_id` can be retrieved after the identity `type` has been set to `SystemAssigned` and Storage Account has been created. More details are available below.

## Attributes Reference

The following attributes are exported in addition to the arguments listed above:
Expand All @@ -150,6 +160,17 @@ The following attributes are exported in addition to the arguments listed above:
* `secondary_connection_string` - The connection string associated with the secondary location
* `primary_blob_connection_string` - The connection string associated with the primary blob location
* `secondary_blob_connection_string` - The connection string associated with the secondary blob location
* `identity` - An `identity` block as defined below, which contains the Identity information for this Storage Account.

---

`identity` exports the following:

* `principal_id` - The Principal ID for the Service Principal associated with the Identity of this Storage Account.

* `tenant_id` - The Tenant ID for the Service Principal associated with the Identity of this Storage Account.

-> You can access the Principal ID via `${azurerm_storage_account.test.identity.0.principal_id}` and the Tenant ID via `${azurerm_storage_account.test.identity.0.tenant_id}`
Copy link
Collaborator

@katbyte katbyte Jun 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need this? There is nothing special required to access these properties

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is still useful for the user to be able to see an example on how to access the property. However, if you still think it's not required - I can remove it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I think it should be removed but its very minor and not a blocker


## Import

Expand Down