From d499e1d85d606baea832f738d5b1060b367b470b Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Thu, 18 Jun 2020 00:14:08 +0100 Subject: [PATCH 1/2] azuread_group: work around replication delay for new members (attempt 10 times, check 10 times) --- azuread/helpers/graph/group.go | 19 +++++++++++++++++-- azuread/helpers/graph/replication.go | 25 +++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/azuread/helpers/graph/group.go b/azuread/helpers/graph/group.go index 325eb19d86..6d5ccc6feb 100644 --- a/azuread/helpers/graph/group.go +++ b/azuread/helpers/graph/group.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "log" + "time" "github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac" ) @@ -121,8 +122,22 @@ func GroupAddMember(client graphrbac.GroupsClient, ctx context.Context, groupId } 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 fmt.Errorf("Error adding group member %q to Azure AD Group with ID %q: %+v", member, groupId, err) + var err error + attempts := 10 + for i := 0; i <= attempts; i++ { + if _, err = client.AddMember(ctx, groupId, properties); err == nil { + break + } + if i == attempts { + return fmt.Errorf("Error adding group member %q to Azure AD Group with ID %q: %+v", member, groupId, err) + } + time.Sleep(time.Second * 2) + } + + if _, err := WaitForListMember(member, func() ([]string, error) { + return GroupAllMembers(client, ctx, groupId) + }); err != nil { + return fmt.Errorf("Error waiting for group membership: %+v", err) } return nil diff --git a/azuread/helpers/graph/replication.go b/azuread/helpers/graph/replication.go index 251e6d3f49..960c2e2d0a 100644 --- a/azuread/helpers/graph/replication.go +++ b/azuread/helpers/graph/replication.go @@ -34,3 +34,28 @@ func WaitForCreationReplication(f func() (interface{}, error)) (interface{}, err }, }).WaitForState() } + +func WaitForListMember(member string, f func() ([]string, error)) (interface{}, error) { + return (&resource.StateChangeConf{ + Pending: []string{"404"}, + Target: []string{"Found"}, + Timeout: 5 * time.Minute, + MinTimeout: 1 * time.Second, + ContinuousTargetOccurence: 10, + Refresh: func() (interface{}, string, error) { + members, err := f() + + if err != nil { + return members, "Error", err + } + + for _, v := range members { + if v == member { + return members, "Found", nil + } + } + + return members, "404", nil + }, + }).WaitForState() +} From 560d87ccd8a7fcdf7db4beca765f1f753fd91572 Mon Sep 17 00:00:00 2001 From: Tom Bamford Date: Thu, 18 Jun 2020 02:43:24 +0100 Subject: [PATCH 2/2] azuread_group: work around replication delays when removing members (attempt 10 times, check 10 times) --- azuread/helpers/graph/group.go | 2 +- azuread/helpers/graph/replication.go | 39 +++++++++++++++++++++++----- azuread/resource_group.go | 21 +++++++++++++-- 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/azuread/helpers/graph/group.go b/azuread/helpers/graph/group.go index 6d5ccc6feb..c22ad79b95 100644 --- a/azuread/helpers/graph/group.go +++ b/azuread/helpers/graph/group.go @@ -134,7 +134,7 @@ func GroupAddMember(client graphrbac.GroupsClient, ctx context.Context, groupId time.Sleep(time.Second * 2) } - if _, err := WaitForListMember(member, func() ([]string, error) { + if _, err := WaitForListAdd(member, func() ([]string, error) { return GroupAllMembers(client, ctx, groupId) }); err != nil { return fmt.Errorf("Error waiting for group membership: %+v", err) diff --git a/azuread/helpers/graph/replication.go b/azuread/helpers/graph/replication.go index 960c2e2d0a..b301d12993 100644 --- a/azuread/helpers/graph/replication.go +++ b/azuread/helpers/graph/replication.go @@ -35,7 +35,7 @@ func WaitForCreationReplication(f func() (interface{}, error)) (interface{}, err }).WaitForState() } -func WaitForListMember(member string, f func() ([]string, error)) (interface{}, error) { +func WaitForListAdd(item string, f func() ([]string, error)) (interface{}, error) { return (&resource.StateChangeConf{ Pending: []string{"404"}, Target: []string{"Found"}, @@ -43,19 +43,44 @@ func WaitForListMember(member string, f func() ([]string, error)) (interface{}, MinTimeout: 1 * time.Second, ContinuousTargetOccurence: 10, Refresh: func() (interface{}, string, error) { - members, err := f() + listItems, err := f() if err != nil { - return members, "Error", err + return listItems, "Error", err } - for _, v := range members { - if v == member { - return members, "Found", nil + for _, v := range listItems { + if v == item { + return listItems, "Found", nil } } - return members, "404", nil + return listItems, "404", nil + }, + }).WaitForState() +} + +func WaitForListRemove(item string, f func() ([]string, error)) (interface{}, error) { + return (&resource.StateChangeConf{ + Pending: []string{"Found"}, + Target: []string{"404"}, + Timeout: 5 * time.Minute, + MinTimeout: 1 * time.Second, + ContinuousTargetOccurence: 10, + Refresh: func() (interface{}, string, error) { + listItems, err := f() + + if err != nil { + return listItems, "Error", err + } + + for _, v := range listItems { + if v == item { + return listItems, "Found", nil + } + } + + return listItems, "404", nil }, }).WaitForState() } diff --git a/azuread/resource_group.go b/azuread/resource_group.go index 17565aac48..0b1d970dde 100644 --- a/azuread/resource_group.go +++ b/azuread/resource_group.go @@ -3,8 +3,10 @@ package azuread import ( "fmt" "log" + "time" "github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac" + "github.com/Azure/go-autorest/autorest" "github.com/google/uuid" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/helper/validation" @@ -186,11 +188,26 @@ func resourceGroupUpdate(d *schema.ResourceData, meta interface{}) error { membersToAdd := slices.Difference(desiredMembers, existingMembers) for _, existingMember := range membersForRemoval { + var err error + var resp autorest.Response 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) { + attempts := 10 + for i := 0; i <= attempts; i++ { + if resp, err = client.RemoveMember(ctx, d.Id(), existingMember); err != nil { + if ar.ResponseWasNotFound(resp) { + break + } + } + if i == attempts { return fmt.Errorf("Error Deleting group member %q from Azure AD Group with ID %q: %+v", existingMember, d.Id(), err) } + time.Sleep(time.Second * 2) + } + + if _, err := graph.WaitForListRemove(existingMember, func() ([]string, error) { + return graph.GroupAllMembers(client, ctx, d.Id()) + }); err != nil { + return fmt.Errorf("Error waiting for group membership removal: %+v", err) } }