azuread_group - support for the owners property

[tiwood]

(1/2 of #36)

This adds a new resource to manage Azure AD Group Owners with Terraform.
Additionally this adds support for owners in the resource azuread_group.

• New resource 'azuread_group_owner'
• New attribute 'owners' for resource 'azuread_group'
• Add tests
• Add documentation
• azuread_group: If owners is not set, it should leave existing owners as is.

Example Usage

resource "azuread_group" "my_group" {
  name = "MyGroup"
}

data "azuread_user" "my_user" {
  user_principal_name = "johndoe@hashicorp.com"
}

resource "azuread_group_owner" "default_owner" {
  group_object_id = "${azuread_group.my_group.id}"
  owner_object_id = "${data.azuread_user.my_user.id}"
}

Example for azuread_group

resource "azuread_group" "my_group" {
  name     = "my group"
  owners  = ["9c9b3992-78ac-11e9-a3ef-a31d2f3e9d2c"] 
}

Argument Reference

The following arguments are supported:

• group_object_id - (Required) The object id of the Azure AD Group where the Owner should be added.
• owner_object_id - (Required) The object id of the Azure AD User you want to add as Owner.

Caveats

Azure requires at least one owner per Azure AD group. That means the destruction of 'azuread_group_owner' resources will fail if no additional owner is present on the group.

[tiwood]

Related: #36

[tiwood]

@katbyte, @tombuildsstuff, do you guys have any ideas how we could tackle the issue of the 'not deletable' (see known issues above) group owners?

BTW you have the same behaviour in the Azure Portal.

1. Create a group
2. Add a new owner
3. Try to remove the owner (this fails if its the last owner of the group)

Should we leave this up to the User?

[twendt]

Would it be an option to not implement it as a separate resource but instead integrate it into the group resource? There you could always pass the full list of owners in and not allow the list to be empty.

[tiwood]

@twendt, I think this is the right way to do it. I'm going to look into it.
Thanks.

[katbyte]

@tiwood & @twendt,

There are reasons to have it as a separate resource (you can separate out creating & managing the owners) & within the resource (central location, ability to explicitly define who the owners should be)

And as such we should support both.

as to the issue of not being able to remove the last user, I say we fail destruction/update and leave that up to the user as theres not much we can do about it.

[tiwood]

I will look into this in the next days.

[katbyte]

Thanks for the new resource @tiwood,

Aside from a few minor comments i've left inline this is looking pretty good! As with the group_user resource i would like to release with support to set this in the adgroup resource aswell. let me know if that is to big of an ask and i'll look into adding that myself 🙂

[katbyte]

@tiwood,

Hope yo don't mind but i've pushed the required changes to this branch to get it good to merge 🙂 LGTM now 👍

[mbfrahry]

LGTM with some minor spelling

[mbfrahry]

Suggested change

	// we could lock here against the group ember resource, but the should not be used together (todo conflicts with at a resource level?)
	// we could lock here against the group member resource, but they should not be used together (todo conflicts with at a resource level?)

[mbfrahry]

Suggested change

	// we could lock here against the group owner resource, but the should not be used together (todo conflicts with at a resource level?)
	// we could lock here against the group owner resource, but they should not be used together (todo conflicts with at a resource level?)

[ghost]

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!