azurerm_app_service - support for MSI
#1130
Conversation
There was a problem hiding this comment.
hey @sabbox
Thanks for this PR. I've taken a look through and this mostly LGTM - if we can add some documentation (and raise the potential error) this should be good to merge :)
Thanks!
| appServiceIdentity := expandAzureRmAppServiceIdentity(d) | ||
| site.Identity = appServiceIdentity | ||
|
|
||
| future, err := client.CreateOrUpdate(ctx, resGroup, name, site) |
There was a problem hiding this comment.
can we handle this error? this is returned for instance when the request is invalid (vs the polling method when the long running request/modifications fail)
| @@ -29,6 +29,33 @@ func resourceArmAppService() *schema.Resource { | |||
| ValidateFunc: validateAppServiceName, | |||
| }, | |||
|
|
|||
| "identity": { | |||
There was a problem hiding this comment.
Can we document these properties in the documentation? That's stored in this file, it'll want the type property documented in the Argument Reference block and the principal_id and tenant_id fields documented in the Attributes Reference block
tombuildsstuff
changed the title
azurerm_app_service - support for MSI
There was a problem hiding this comment.
hey @sabbox
Thanks for pushing those updates - apologies for the delayed re-review here!
I've pushed a couple of commits to inline the separate updateAppService method and make the documentation consistent - but this otherwise LGTM, so I'll kick off the test suite now 👍
Thanks!
|
Tests pass:
|
|
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 👉 hashibot-feedback@hashicorp.com. Thanks! |
ghost
locked and limited conversation to collaborators
This PR enables Managed Service Identity for App Service #1106