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

azurerm_function_app: Add support for user assigned identities #4687

Conversation

joakimhew
Copy link
Contributor

@joakimhew joakimhew commented Oct 22, 2019

This PR fixes #4607 and allows users to add user assigned identities to the identities block.

User assigned identities are defined like so:

identity {
   type = "UserAssigned"

   user_assigned_identity {
      id = "${azurerm_user_assigned_identity.this.id}"
   }
 }

@JayDoubleu
Copy link

Any chance to get this PR in before next provider release ? It's a very useful feature.

@JayDoubleu
Copy link

JayDoubleu commented Oct 27, 2019

@joakimhew

identity {
    type         = "UserAssigned"
    identity_ids = []
  }

Is this going to work like with multiple User managed MSIs like with virtual_machine for example?
https://www.terraform.io/docs/providers/azurerm/r/virtual_machine.html#identity
I noticed your example from above doesn't mention list/

I'm assuming this would use the block style instead?

identity {
   type = "UserAssigned"

   user_assigned_identity {
      id = "${azurerm_user_assigned_identity.first.id}"
   }
   user_assigned_identity {
      id = "${azurerm_user_assigned_identity.second.id}"
   }
 }

@joakimhew
Copy link
Contributor Author

Is this going to work like with multiple User managed MSIs like with virtual_machine for example?

I'm assuming this would use the block style instead?

identity {
   type = "UserAssigned"

   user_assigned_identity {
      id = "${azurerm_user_assigned_identity.first.id}"
   }
   user_assigned_identity {
      id = "${azurerm_user_assigned_identity.second.id}"
   }
 }

@JayDoubleu Exactly!
So an example for defining multiple policies would be:

resource "azurerm_user_assigned_identity" "first" {
  location            = "westus"
  name                = "first"
}

resource "azurerm_user_assigned_identity" "second" {
  location            = "westus"
  name                = "second"
}

resource "azurerm_function_app" "test" {
  name     = "functionapp"
  location = "westus"

  identity {
    type = "UserAssigned"
    user_assigned_identity {
      id = "${azurerm_user_assigned_identity.first.id}"
    }
    user_assigned_identity {
      id = "${azurerm_user_assigned_identity.second.id}"
    }
  }
}

Then to access the computed principal id and client id:

azurerm_function_app.test.user_assigned_identity.0.principal_id
azurerm_function_app.test.user_assigned_identity.0.client_id
azurerm_function_app.test.user_assigned_identity.1.principal_id
azurerm_function_app.test.user_assigned_identity.1.client_id

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @joakimhellum

Thanks for this PR :)

Taking a look through this is looking good - if we can fix up the minor comments (and the tests pass) then this otherwise LGTM 👍

Thanks!

azurerm/resource_arm_function_app.go Outdated Show resolved Hide resolved
azurerm/resource_arm_function_app.go Outdated Show resolved Hide resolved
azurerm/resource_arm_function_app.go Outdated Show resolved Hide resolved
azurerm/resource_arm_function_app.go Outdated Show resolved Hide resolved
website/docs/r/function_app.html.markdown Show resolved Hide resolved
@JayDoubleu
Copy link

Is there any movement on this? I believe it already missed 2 releases. Is it likely to be merged for next one ?

@tombuildsstuff tombuildsstuff modified the milestones: v1.38.0, v1.39.0 Dec 3, 2019
@tombuildsstuff tombuildsstuff self-assigned this Dec 5, 2019
@tombuildsstuff tombuildsstuff modified the milestones: v1.39.0, v1.40.0 Dec 12, 2019
@joakimhew
Copy link
Contributor Author

@JayDoubleu I think the team has a lot of things to do. @tombuildsstuff Is there anything else you need from me? As far as I can see, the PR is ready do be merged?

@tombuildsstuff tombuildsstuff removed their assignment Dec 18, 2019
@tombuildsstuff tombuildsstuff self-assigned this Dec 19, 2019
@tombuildsstuff
Copy link
Contributor

@joakimhellum sorry for the delay on this - taking a look at this today 👍

@tombuildsstuff tombuildsstuff modified the milestones: v1.40.0, v1.41.0 Dec 19, 2019
@ykulinichenko
Copy link

Will this be included into version 1.41.0? Really need this feature a lot :)

@tombuildsstuff tombuildsstuff modified the milestones: v1.41.0, v1.42.0 Jan 14, 2020
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

Hi @joakimhew,

thank you for this PR, my main ask is to make this consistent with how we want identity blocks to become across the provider:

identity {
    type = "UserAssigned"
    identity_ids = [
      azurerm_user_assigned_identity.test.id,
    ]
  }

app service, app gateway and container group all do things the desired way. Thanks!

@@ -153,6 +155,27 @@ func resourceArmFunctionApp() *schema.Resource {
Type: schema.TypeString,
Computed: true,
},
"user_assigned_identity": {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are trying to make these identity blocks consistent. So like app service, app gateway and container group could we make this:

	identity_ids = [
      azurerm_user_assigned_identity.test.id,
    ]

and then we can reuse the principal ID property above

@tombuildsstuff tombuildsstuff removed their assignment Jan 23, 2020
@tombuildsstuff tombuildsstuff modified the milestones: v1.42.0, v1.43.0 Jan 24, 2020
@tombuildsstuff tombuildsstuff modified the milestones: v1.43.0, v1.44.0 Feb 4, 2020
tombuildsstuff added a commit that referenced this pull request Feb 11, 2020
Co-authored-by: Joakim Hansson <[email protected]>

Porting-over the work done by @joakimhew in #4687 before the refactor
@tombuildsstuff
Copy link
Contributor

hey @joakimhew

I've taken a look into rebasing/fixing up this PR in order to to get this merged - however since these commits were made from prior to the refactor this is in a position where rebasing this is going to be impractical. As such I hope you don't mind but I'm going to close this in favour of #5676 which implements this functionality and lists you as a co-author (so you'll still get the credit for adding this functionality).

Thanks!

@ghost
Copy link

ghost commented Feb 12, 2020

This has been released in version 1.44.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 1.44.0"
}
# ... other configuration ...

@ghost ghost removed the waiting-response label Feb 12, 2020
@ghost
Copy link

ghost commented Mar 28, 2020

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!

@ghost ghost locked and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

azurerm_function_app - Add support for UserAssigned identity
5 participants