Storage Account: Add identity property #1323

lstolyarov · 2018-05-31T16:10:23Z

This PR adds the object id as an output of a storage account. At the moment resp.Identity is returning nil. Any ideas why?

tombuildsstuff · 2018-05-31T16:55:46Z

azurerm/resource_arm_storage_account.go

@@ -495,6 +500,11 @@ func resourceArmStorageAccountRead(d *schema.ResourceData, meta interface{}) err
 	}
 	d.Set("account_kind", resp.Kind)

+	log.Printf("[INFO] Identity is %q", resp.Identity)
+	if identity := resp.Identity; identity != nil {


@lstolyarov you'll want to use resp.[XX]Properties.Identity here - since the top level fields resp.Identity isn't guaranteed to have a value, unfortunately. These fields exist primarily for the older API's where the responses aren't guaranteed in the properties block in the JSON

@tombuildsstuff what do you mean by [XX]Properties? There is an AccountProperties filed in resp but that does not have an Identity field

@tombuildsstuff what do you mean by [XX]Properties? There is an AccountProperties filed in resp but that does not have an Identity field

The properties field is prefixed with the name of the struct, so it's different (hence [XX]Properties, sorry I should have looked this up).

I don't see any option to enable this in the Portal - is this feature in Preview?

Well it is not really a feature. This PR is to be able to extract the Object ID (or Principal ID as it is referred to in the SDK). I don't think there is any anyway to get this information out in the portal - it could be extracted with powershell.

sorry, I was referring to pulling this out via the API/SDK - I'm not sure if you've seen it but the Azure Resources Explorer can be really helpful here.

In other API's (e.g. App Service) the identity block won't be returned from the API until it's enabled, which can be done by sending the following Request (in the Create/Update):

Identity: &storage.Identity{ Type: utils.String("SystemAssigned"), },

Once the Identity's assigned, the identity block returned from the API SDK as Principal ID/Object ID. That said, given this isn't available in the Portal yet - and I can't see any reference to it online; I have a feeling this may be in Preview?

Ok, that makes more sense now.

Thanks for the Azure Resources Explorer - its really useful.

I am not sure if it would technically count as a preview feature or not. My understanding that behind the scenes all resources have object ids and this is required as input for key vault. It is not really useful in the portal as when you enable storage account encryption and select the key vault you want to use that object id gets populated automatically.

tombuildsstuff · 2018-05-31T17:58:13Z

Related issue: #658

katbyte

Hello @lstolyarov,

Thank you for opening this PR. It mostly LGTM and I've let a couple of comments inline. Once those are sorted I look forward to merging this for you 🙂

katbyte · 2018-06-06T01:43:17Z

azurerm/resource_arm_storage_account.go

+	}
+
+	result := make(map[string]interface{})
+	result["type"] = *identity.Type


Could we also check identity.Type for nil?

katbyte · 2018-06-06T01:44:19Z

website/docs/r/storage_account.html.markdown

+
+* `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}`


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

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.

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

katbyte · 2018-06-06T01:48:17Z

website/docs/r/storage_account.html.markdown

+
+* `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 Storage Account has been created. More details are available below.


Might it be worth explaining these properties won't be assigned until the identity type value is set to SystemAssigned ?

virtualbubble · 2018-06-07T15:00:14Z

Any idea on when this will be released?

katbyte

Hi @lstolyarov,

Thank you for the updates, this LGTM now however running the tests I am consistently getting this failure:

------- Stdout: -------
=== RUN   TestAccAzureRMStorageAccount_updateResourceByEnablingIdentity
--- FAIL: TestAccAzureRMStorageAccount_updateResourceByEnablingIdentity (93.41s)
    testing.go:513: Step 1 error: Check failed: Check 3/4 error: azurerm_storage_account.testsa: Attribute 'identity.0.principal_id' didn't match "^[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}$", got ""
FAIL

Additionally, could you please merge/rebase to resolve the merge conflicts?

lstolyarov · 2018-06-13T14:53:22Z

Hey @katbyte,

This should be ready for another review now. Tests are passing & merge conflicts are resolved.

Removed some lines that were accidentally merged back in.

katbyte

Hi @lstolyarov,

Thank you for the updates, LGTM 👍

fix go fmt issues

katbyte · 2018-06-14T03:02:18Z

tests pass (failing ones are encryption and too many networks)

ghost · 2020-03-30T15:35:40Z

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!

Add the storage account object id as an output

ac0ccbe

lstolyarov changed the title ~~[WIP] Add the storage account object id as an output~~ [WIP] Storage Account: Add object id as an output May 31, 2018

tombuildsstuff reviewed May 31, 2018

View reviewed changes

tombuildsstuff added enhancement service/storage labels May 31, 2018

leostolyarov added 3 commits June 2, 2018 13:26

Update the storage account resource to include identity

68d994d

Add the storage account identity tests

28f4b30

Update the storage account identity block documentation

c3f58da

lstolyarov changed the title ~~[WIP] Storage Account: Add object id as an output~~ Storage Account: Add object id as an output Jun 4, 2018

katbyte requested changes Jun 6, 2018

View reviewed changes

katbyte reviewed Jun 6, 2018

View reviewed changes

leostolyarov added 2 commits June 8, 2018 09:40

Checking identity type is not nill

f7df70d

Updating storage account identity documentation

8dd38e2

katbyte requested changes Jun 8, 2018

View reviewed changes

katbyte changed the title ~~Storage Account: Add object id as an output~~ Storage Account: Add identity property Jun 8, 2018

leostolyarov and others added 3 commits June 13, 2018 15:35

Updating to make the updateResourceByEnablingIdentity test pass

8a2457e

Merge branch 'master' into storage-account-identity

0b448c8

Running fmt

cb8dd58

Update resource_arm_storage_account.go

a0db137

Removed some lines that were accidentally merged back in.

katbyte approved these changes Jun 14, 2018

View reviewed changes

Update resource_arm_storage_account.go

86fe88c

fix go fmt issues

katbyte merged commit efc4bc5 into hashicorp:master Jun 14, 2018

katbyte added a commit that referenced this pull request Jun 14, 2018

Update CHANGELOG.md to include #1323

8fd768c

bg2132332 mentioned this pull request Jul 13, 2018

Encryption Settings - Storage Account #658

Closed

ghost locked and limited conversation to collaborators Mar 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Storage Account: Add identity property #1323

Storage Account: Add identity property #1323

lstolyarov commented May 31, 2018

tombuildsstuff May 31, 2018

lstolyarov Jun 1, 2018

tombuildsstuff Jun 1, 2018

lstolyarov Jun 1, 2018

tombuildsstuff Jun 1, 2018

lstolyarov Jun 2, 2018

tombuildsstuff commented May 31, 2018

katbyte left a comment

katbyte Jun 6, 2018

katbyte Jun 6, 2018 •

edited

Loading

lstolyarov Jun 8, 2018

katbyte Jun 8, 2018

katbyte Jun 6, 2018

virtualbubble commented Jun 7, 2018

katbyte left a comment •

edited

Loading

lstolyarov commented Jun 13, 2018

katbyte left a comment

katbyte commented Jun 14, 2018

ghost commented Mar 30, 2020


		* `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}`


		* `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 Storage Account has been created. More details are available below.

Storage Account: Add identity property #1323

Storage Account: Add identity property #1323

Conversation

lstolyarov commented May 31, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tombuildsstuff commented May 31, 2018

katbyte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katbyte Jun 6, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

virtualbubble commented Jun 7, 2018

katbyte left a comment • edited Loading

Choose a reason for hiding this comment

lstolyarov commented Jun 13, 2018

katbyte left a comment

Choose a reason for hiding this comment

katbyte commented Jun 14, 2018

ghost commented Mar 30, 2020

katbyte Jun 6, 2018 •

edited

Loading

katbyte left a comment •

edited

Loading