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

Resource: 'azuread_group_member' #100

Merged
merged 25 commits into from
Jul 12, 2019
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
16cbd0d
New resource 'azuread_group_member'
tiwood Mar 13, 2019
61312b7
Tests for 'azuread_group_member'
tiwood Mar 13, 2019
80a03d3
Documentation for new resource 'azuread_group_member'
tiwood Mar 13, 2019
68c6cae
resource_group_member: fix review comments
evenh Jun 5, 2019
b1a030c
resource_group: add members
evenh Jun 5, 2019
192c04b
resource_group: documentation
evenh Jun 6, 2019
6f0be29
resource_group: add test for members
evenh Jun 6, 2019
b1dd10d
fix lint issues
evenh Jun 6, 2019
fd1b4b7
resource_group: extract fetchAllMembers into graph helper
evenh Jun 13, 2019
5d32e56
resource_group: extract addMember into graph helper
evenh Jun 13, 2019
500b2a9
resource_group: bugfix for empty set of members
evenh Jun 13, 2019
4d6e3f9
Misc fixes
evenh Jun 17, 2019
cc51f50
resource_group: fix broken acceptance tests
evenh Jun 19, 2019
953508d
resource_group: mark 'members' as computed
evenh Jun 19, 2019
61b9536
resource_group: more acceptance tests
evenh Jun 24, 2019
b6fdf5f
resource_group: fix linter warning in acceptance test
evenh Jun 24, 2019
c86d8c6
resource_group: id -> object_id in tests
evenh Jun 24, 2019
6c097b1
graph/group: change method signatures
evenh Jun 25, 2019
646fa01
graph/group: introduce GroupAddMembers
evenh Jun 25, 2019
bbdc1a5
resource_group: use `member` as an ID separator
evenh Jun 25, 2019
3c17afc
update documentation
evenh Jun 25, 2019
702e869
resource_group: test review comments
evenh Jun 25, 2019
28ac563
resource_group: revert to plain HCL for tests
evenh Jun 26, 2019
aeba0ad
resource_group: Use GroupAddMembers helper
evenh Jun 28, 2019
dbde7fb
graph/group: wrap errors
evenh Jun 28, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions azuread/helpers/slices/slices.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package slices

// difference returns the elements in `a` that aren't in `b`.
func Difference(a, b []string) []string {
mb := make(map[string]struct{}, len(b))
for _, x := range b {
mb[x] = struct{}{}
}
var diff []string
for _, x := range a {
if _, found := mb[x]; !found {
diff = append(diff, x)
}
}
return diff
}
1 change: 1 addition & 0 deletions azuread/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ func Provider() terraform.ResourceProvider {
"azuread_application": resourceApplication(),
"azuread_application_password": resourceApplicationPassword(),
"azuread_group": resourceGroup(),
"azuread_group_member": resourceGroupMember(),
"azuread_service_principal": resourceServicePrincipal(),
"azuread_service_principal_password": resourceServicePrincipalPassword(),
"azuread_user": resourceUser(),
Expand Down
106 changes: 106 additions & 0 deletions azuread/resource_group.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
package azuread

import (
"context"
"fmt"
"log"

"github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/slices"
"github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/tf"
"github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/validate"

"github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac"
"github.com/google/uuid"
"github.com/hashicorp/terraform/helper/schema"
Expand All @@ -17,6 +22,7 @@ func resourceGroup() *schema.Resource {
return &schema.Resource{
Create: resourceGroupCreate,
Read: resourceGroupRead,
Update: resourceGroupUpdate,
Delete: resourceGroupDelete,

Importer: &schema.ResourceImporter{
Expand All @@ -35,6 +41,17 @@ func resourceGroup() *schema.Resource {
Type: schema.TypeString,
Computed: true,
},

"members": {
Type: schema.TypeSet,
Optional: true,
Set: schema.HashString,
ForceNew: false,
Elem: &schema.Schema{
Type: schema.TypeString,
ValidateFunc: validate.UUID,
},
},
},
}
}
Expand All @@ -61,6 +78,17 @@ func resourceGroupCreate(d *schema.ResourceData, meta interface{}) error {
}
d.SetId(*group.ObjectID)

// Add members if specified
if v, ok := d.GetOk("members"); ok {
members := tf.ExpandStringSlicePtr(v.(*schema.Set).List())

for _, memberUuid := range *members {
evenh marked this conversation as resolved.
Show resolved Hide resolved
if err := addMember(*group.ObjectID, memberUuid, client, ctx); err != nil {
return err
}
}
}

_, err = graph.WaitForReplication(func() (interface{}, error) {
return client.Get(ctx, *group.ObjectID)
})
Expand Down Expand Up @@ -88,9 +116,50 @@ func resourceGroupRead(d *schema.ResourceData, meta interface{}) error {

d.Set("name", resp.DisplayName)
d.Set("object_id", resp.ObjectID)

members, err := fetchAllMembers(d.Id(), client, ctx)
evenh marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return err
}

d.Set("members", members)

return nil
}

func resourceGroupUpdate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*ArmClient).groupsClient
ctx := meta.(*ArmClient).StopContext

if v, ok := d.GetOk("members"); ok && d.HasChange("members") {
existingMembers, err := fetchAllMembers(d.Id(), client, ctx)
if err != nil {
return err
}

desiredMembers := *tf.ExpandStringSlicePtr(v.(*schema.Set).List())
membersForRemoval := slices.Difference(existingMembers, desiredMembers)
membersToAdd := slices.Difference(desiredMembers, existingMembers)

for _, existingMember := range membersForRemoval {
log.Printf("[DEBUG] Removing member with id %q from Azure AD group with id %q", existingMember, d.Id())
if resp, err := client.RemoveMember(ctx, d.Id(), existingMember); err != nil {
if !ar.ResponseWasNotFound(resp) {
return fmt.Errorf("Error Deleting group member %q from Azure AD Group with ID %q: %+v", existingMember, d.Id(), err)
}
}
}

for _, newMember := range membersToAdd {
evenh marked this conversation as resolved.
Show resolved Hide resolved
if err := addMember(d.Id(), newMember, client, ctx); err != nil {
return err
evenh marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

return resourceGroupRead(d, meta)
}

func resourceGroupDelete(d *schema.ResourceData, meta interface{}) error {
client := meta.(*ArmClient).groupsClient
ctx := meta.(*ArmClient).StopContext
Expand All @@ -103,3 +172,40 @@ func resourceGroupDelete(d *schema.ResourceData, meta interface{}) error {

return nil
}

func fetchAllMembers(groupId string, client graphrbac.GroupsClient, ctx context.Context) ([]string, error) {
it, err := client.GetGroupMembersComplete(ctx, groupId)

if err != nil {
return nil, fmt.Errorf("Error listing existing group members from Azure AD Group with ID %q: %+v", groupId, err)
}

existingMembers := make([]string, 0)

for it.NotDone() {
currUser, _ := it.Value().AsUser()
evenh marked this conversation as resolved.
Show resolved Hide resolved
existingMembers = append(existingMembers, *currUser.ObjectID)
if err := it.NextWithContext(ctx); err != nil {
return nil, fmt.Errorf("Error during pagination of group members from Azure AD Group with ID %q: %+v", groupId, err)
}
}

log.Printf("[DEBUG] %d members in Azure AD group with ID: %q", len(existingMembers), groupId)

return existingMembers, nil
}

func addMember(groupId string, member string, client graphrbac.GroupsClient, ctx context.Context) error {
evenh marked this conversation as resolved.
Show resolved Hide resolved
memberGraphURL := fmt.Sprintf("https://graph.windows.net/%s/directoryObjects/%s", client.TenantID, member)

properties := graphrbac.GroupAddMemberParameters{
URL: &memberGraphURL,
}

log.Printf("[DEBUG] Adding member with id %q to Azure AD group with id %q", member, groupId)
if _, err := client.AddMember(ctx, groupId, properties); err != nil {
return err
}

return nil
}
151 changes: 151 additions & 0 deletions azuread/resource_group_member.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
package azuread

import (
"fmt"
"strings"

"github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac"
"github.com/hashicorp/terraform/helper/schema"
"github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar"
"github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/validate"
)

func resourceGroupMember() *schema.Resource {
return &schema.Resource{
Create: resourceGroupMemberCreate,
Read: resourceGroupMemberRead,
Delete: resourceGroupMemberDelete,
Importer: &schema.ResourceImporter{
State: schema.ImportStatePassthrough,
},

Schema: map[string]*schema.Schema{
"group_object_id": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validate.UUID,
},
"member_object_id": {
Type: schema.TypeString,
Required: true,
ForceNew: true,
ValidateFunc: validate.UUID,
},
},
}
}

func resourceGroupMemberCreate(d *schema.ResourceData, meta interface{}) error {
client := meta.(*ArmClient).groupsClient
ctx := meta.(*ArmClient).StopContext

groupID := d.Get("group_object_id").(string)
memberID := d.Get("member_object_id").(string)
tenantID := client.TenantID

memberGraphURL := fmt.Sprintf("https://graph.windows.net/%s/directoryObjects/%s", tenantID, memberID)

properties := graphrbac.GroupAddMemberParameters{
URL: &memberGraphURL,
}

if _, err := client.AddMember(ctx, groupID, properties); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this fail if the member already exists?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we also reuse the graph.GroupAddMember function discussed above?

Copy link
Contributor Author

@evenh evenh Jun 18, 2019

Choose a reason for hiding this comment

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

I've refactored to use the new helper function. However, if a member already exists it fails. What is the desired outcome here? Given the following HCL:

// User one already exists
resource "azuread_group" "some_group" {
  name = "SomeGroup"
}

resource "azuread_group_member" "one" {
  group_object_id   = azuread_group.some_group.id
  member_object_id  = azuread_user.one.id
}

resource "azuread_group_member" "another" {
  group_object_id   = azuread_group.some_group.id
  member_object_id  = azuread_user.one.id
}

In that case Terraform sees two "new" resource in azuread_group_member.one and azuread_group_member.another. After applying the first one, another fails with

graphrbac.GroupsClient#AddMember: Failure responding to request: StatusCode=400 -- Original Error: autorest/azure: Service returned an error. Status=400 Code="Unknown" Message="Unknown service error" Details=[{"odata.error":{"code":"Request_BadRequest","date":"2019-06-18T11:09:20","message":{"lang":"en","value":"One or more added object references already exist for the following modified properties: 'members'."},"requestId":"c7ad6982-559e-46eb-bfed-56b6455141c8"}}]

return err
}

id := fmt.Sprintf("%s/%s", groupID, memberID)
evenh marked this conversation as resolved.
Show resolved Hide resolved
d.SetId(id)

return resourceGroupMemberRead(d, meta)
}

func resourceGroupMemberRead(d *schema.ResourceData, meta interface{}) error {
client := meta.(*ArmClient).groupsClient
ctx := meta.(*ArmClient).StopContext

id := strings.Split(d.Id(), "/")
if len(id) != 2 {
return fmt.Errorf("ID should be in the format {groupObjectId}/{memberObjectId} - but got %q", d.Id())
evenh marked this conversation as resolved.
Show resolved Hide resolved
}

groupID := id[0]
memberID := id[1]

members, err := client.GetGroupMembersComplete(ctx, groupID)
if err != nil {
return fmt.Errorf("Error retrieving Azure AD Group members (groupObjectId: %q): %+v", groupID, err)
}

var memberObjectID string
for members.NotDone() {
// possible members are users, groups or service principals
// we try to 'cast' each result as the corresponding type and diff
// if we found the object we're looking for
user, _ := members.Value().AsUser()
if user != nil {
if *user.ObjectID == memberID {
memberObjectID = *user.ObjectID
// we successfully found the directory object we're looking for, we can stop looping
// through the results
break
}
}

group, _ := members.Value().AsADGroup()
if group != nil {
if *group.ObjectID == memberID {
memberObjectID = *group.ObjectID
// we successfully found the directory object we're looking for, we can stop looping
// through the results
break
}
}

servicePrincipal, _ := members.Value().AsServicePrincipal()
if servicePrincipal != nil {
if *servicePrincipal.ObjectID == memberID {
memberObjectID = *servicePrincipal.ObjectID
// we successfully found the directory object we're looking for, we can stop looping
// through the results
break
}
}

if err = members.NextWithContext(ctx); err != nil {
return fmt.Errorf("Error listing Azure AD Group Members: %s", err)
}
}

if memberObjectID == "" {
d.SetId("")
return fmt.Errorf("Azure AD Group Member not found - groupObjectId:%q / memberObjectId:%q", groupID, memberID)
}

d.Set("group_object_id", groupID)
d.Set("member_object_id", memberObjectID)

return nil
}

func resourceGroupMemberDelete(d *schema.ResourceData, meta interface{}) error {
client := meta.(*ArmClient).groupsClient
ctx := meta.(*ArmClient).StopContext

id := strings.Split(d.Id(), "/")
if len(id) != 2 {
return fmt.Errorf("ID should be in the format {groupObjectId}/{memberObjectId} - but got %q", d.Id())
}

groupID := id[0]
memberID := id[1]

resp, err := client.RemoveMember(ctx, groupID, memberID)
if err != nil {
if !ar.ResponseWasNotFound(resp) {
return fmt.Errorf("Error removing Member (memberObjectId: %q) from Azure AD Group (groupObjectId: %q): %+v", memberID, groupID, err)
}
}

return nil
}
Loading