Skip to content

Commit

Permalink
Merge pull request #1090 from hashicorp/bugfix/client-retry-inheritence
Browse files Browse the repository at this point in the history
Bugfix: mitigate inheritence or long race condition between resource functions when clients have retries disabled
  • Loading branch information
manicminer authored Apr 27, 2023
2 parents a7b92f9 + 24f5845 commit 3ac0b3a
Show file tree
Hide file tree
Showing 65 changed files with 77 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ func administrativeUnitMemberResourceDelete(ctx context.Context, d *schema.Resou

// Wait for membership link to be deleted
if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
if _, status, err := client.GetMember(ctx, id.AdministrativeUnitId, id.MemberId); err != nil {
if status == http.StatusNotFound {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ func TestAccAdministrativeUnitMember_requiresImport(t *testing.T) {
func (r AdministrativeUnitMemberResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.AdministrativeUnits.AdministrativeUnitsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

id, err := parse.AdministrativeUnitMemberID(state.ID)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ func administrativeUnitResourceDelete(ctx context.Context, d *schema.ResourceDat

// Wait for administrative unit object to be deleted
if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
if _, status, err := client.Get(ctx, administrativeUnitId, odata.Query{}); err != nil {
if status == http.StatusNotFound {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ func TestAccGroup_preventDuplicateNamesFail(t *testing.T) {
func (r AdministrativeUnitResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.AdministrativeUnits.AdministrativeUnitsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

role, status, err := client.Get(ctx, state.ID, odata.Query{})
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ func applicationCertificateResourceDelete(ctx context.Context, d *schema.Resourc

// Wait for application certificate to be deleted
if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true

app, _, err := client.Get(ctx, id.ObjectId, odata.Query{})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ func TestAccApplicationCertificate_requiresImport(t *testing.T) {
func (ApplicationCertificateResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.Applications.ApplicationsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

id, err := parse.CertificateID(state.ID)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions internal/services/applications/application_data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -495,6 +495,7 @@ func applicationDataSource() *schema.Resource {
func applicationDataSourceRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
client := meta.(*clients.Client).Applications.ApplicationsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

var app *msgraph.Application

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ func applicationFederatedIdentityCredentialResourceDelete(ctx context.Context, d

// Wait for credential to be deleted
if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true

credentials, _, err := client.ListFederatedIdentityCredentials(ctx, id.ObjectId, odata.Query{})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ func TestAccApplicationFederatedIdentityCredential_update(t *testing.T) {
func (r ApplicationFederatedIdentityCredentialResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.Applications.ApplicationsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

id, err := parse.FederatedIdentityCredentialID(state.ID)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,7 @@ func applicationPasswordResourceDelete(ctx context.Context, d *schema.ResourceDa

// Wait for application password to be deleted
if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true

app, _, err := client.Get(ctx, id.ObjectId, odata.Query{})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ func TestAccApplicationPassword_relativeEndDate(t *testing.T) {
func (r ApplicationPasswordResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.Applications.ApplicationsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

id, err := parse.PasswordID(state.ID)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ func TestAccApplicationPreAuthorized_requiresImport(t *testing.T) {
func (ApplicationPreAuthorizedResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.Applications.ApplicationsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

id, err := parse.ApplicationPreAuthorizedID(state.ID)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions internal/services/applications/application_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,7 @@ func applicationResourceDelete(ctx context.Context, d *schema.ResourceData, meta

// Wait for application object to be deleted
if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
if _, status, err := client.Get(ctx, appId, odata.Query{}); err != nil {
if status == http.StatusNotFound {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,7 @@ func TestAccApplication_logo(t *testing.T) {
func (r ApplicationResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.Applications.ApplicationsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()
app, status, err := client.Get(ctx, state.ID, odata.Query{})
if err != nil {
if status == http.StatusNotFound {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ func applicationTemplateDataSource() *schema.Resource {
func applicationTemplateDataSourceRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
client := meta.(*clients.Client).Applications.ApplicationTemplatesClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

var template *msgraph.ApplicationTemplate

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ func TestAccAppRoleAssignment_userForTenantApp(t *testing.T) {
func (r AppRoleAssignmentResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.AppRoleAssignments.AppRoleAssignedToClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

id, err := parse.AppRoleAssignmentID(state.ID)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,7 @@ func conditionalAccessPolicyResourceDelete(ctx context.Context, d *schema.Resour
}

if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
if _, status, err := client.Get(ctx, policyId, odata.Query{}); err != nil {
if status == http.StatusNotFound {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,7 @@ func namedLocationResourceDelete(ctx context.Context, d *schema.ResourceData, me
}

if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
if _, status, err := client.Get(ctx, namedLocationId, odata.Query{}); err != nil {
if status == http.StatusNotFound {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ func directoryObjectDataSource() *schema.Resource {
func directoryObjectDataSourceRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
client := meta.(*clients.Client).Users.DirectoryObjectsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

var directoryObject *msgraph.DirectoryObject

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ func TestAccCustomDirectoryRole_templateId(t *testing.T) {
func (r CustomDirectoryRoleResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.DirectoryRoles.RoleDefinitionsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

role, status, err := client.Get(ctx, state.ID, odata.Query{})
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ func TestAccDirectoryRoleAssignment_multipleUser(t *testing.T) {
func (r DirectoryRoleAssignmentResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.DirectoryRoles.RoleAssignmentsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

if _, status, err := client.Get(ctx, state.ID, odata.Query{}); err != nil {
if status == http.StatusNotFound {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,7 @@ func directoryRoleMemberResourceDelete(ctx context.Context, d *schema.ResourceDa

// Wait for membership link to be deleted
if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
if _, status, err := client.GetMember(ctx, id.DirectoryRoleId, id.MemberId); err != nil {
if status == http.StatusNotFound {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ func TestAccDirectoryRoleMember_requiresImport(t *testing.T) {
func (r DirectoryRoleMemberResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.DirectoryRoles.DirectoryRolesClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

id, err := parse.DirectoryRoleMemberID(state.ID)
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ func TestAccDirectoryRole_byTemplateId(t *testing.T) {
func (r DirectoryRoleResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.DirectoryRoles.DirectoryRolesClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

role, status, err := client.Get(ctx, state.ID)
if err != nil {
Expand Down
2 changes: 2 additions & 0 deletions internal/services/domains/domains_data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,8 @@ func domainsDataSource() *schema.Resource {
func domainsDataSourceRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
client := meta.(*clients.Client).Domains.DomainsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

tenantId := meta.(*clients.Client).TenantID

adminManaged := d.Get("admin_managed").(bool)
Expand Down
1 change: 1 addition & 0 deletions internal/services/groups/group_data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ func groupDataSource() *schema.Resource {
func groupDataSourceRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
client := meta.(*clients.Client).Groups.GroupsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

var group msgraph.Group
var displayName string
Expand Down
1 change: 1 addition & 0 deletions internal/services/groups/group_member_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ func groupMemberResourceDelete(ctx context.Context, d *schema.ResourceData, meta

// Wait for membership link to be deleted
if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
if _, status, err := client.GetMember(ctx, id.GroupId, id.MemberId); err != nil {
if status == http.StatusNotFound {
Expand Down
1 change: 1 addition & 0 deletions internal/services/groups/group_member_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ func TestAccGroupMember_requiresImport(t *testing.T) {
func (r GroupMemberResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.Groups.GroupsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

id, err := parse.GroupMemberID(state.ID)
if err != nil {
Expand Down
12 changes: 12 additions & 0 deletions internal/services/groups/group_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -727,6 +727,7 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter

// Wait for DisplayName to be updated
if err := helpers.WaitForUpdate(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
group, status, err := client.Get(ctx, *group.ID(), odata.Query{})
if err != nil {
Expand All @@ -746,6 +747,7 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter
if description == "" {
// Ignoring the error result here because the description might not be updated out of band, in which case we skip over this
if updated, _ := helpers.WaitForUpdateWithTimeout(ctx, 2*time.Minute, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
group, _, err := client.Get(ctx, *group.ID(), odata.Query{})
if err != nil {
Expand All @@ -768,6 +770,7 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter

// Wait for Description to be removed
if err = helpers.WaitForUpdate(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
group, _, err = client.Get(ctx, *group.ID(), odata.Query{})
if err != nil {
Expand Down Expand Up @@ -798,6 +801,7 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter

// Wait for AllowExternalSenders to be updated
if err := helpers.WaitForUpdate(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
groupExtra, err := groupGetAdditional(ctx, client, *group.ID())
if err != nil {
Expand All @@ -822,6 +826,7 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter

// Wait for AutoSubscribeNewMembers to be updated
if err := helpers.WaitForUpdate(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
groupExtra, err := groupGetAdditional(ctx, client, *group.ID())
if err != nil {
Expand All @@ -846,6 +851,7 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter

// Wait for HideFromAddressLists to be updated
if err := helpers.WaitForUpdate(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
groupExtra, err := groupGetAdditional(ctx, client, *group.ID())
if err != nil {
Expand All @@ -870,6 +876,7 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter

// Wait for HideFromOutlookClients to be updated
if err := helpers.WaitForUpdate(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
groupExtra, err := groupGetAdditional(ctx, client, *group.ID())
if err != nil {
Expand Down Expand Up @@ -1032,6 +1039,7 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter

// Wait for AllowExternalSenders to be updated
if err := helpers.WaitForUpdate(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
groupExtra, err := groupGetAdditional(ctx, client, *group.ID())
if err != nil {
Expand All @@ -1056,6 +1064,7 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter

// Wait for AutoSubscribeNewMembers to be updated
if err := helpers.WaitForUpdate(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
groupExtra, err := groupGetAdditional(ctx, client, *group.ID())
if err != nil {
Expand All @@ -1080,6 +1089,7 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter

// Wait for HideFromAddressLists to be updated
if err := helpers.WaitForUpdate(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
groupExtra, err := groupGetAdditional(ctx, client, *group.ID())
if err != nil {
Expand All @@ -1104,6 +1114,7 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter

// Wait for HideFromOutlookClients to be updated
if err := helpers.WaitForUpdate(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
groupExtra, err := groupGetAdditional(ctx, client, *group.ID())
if err != nil {
Expand Down Expand Up @@ -1383,6 +1394,7 @@ func groupResourceDelete(ctx context.Context, d *schema.ResourceData, meta inter

// Wait for group object to be deleted
if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
if _, status, err := client.Get(ctx, groupId, odata.Query{}); err != nil {
if status == http.StatusNotFound {
Expand Down
1 change: 1 addition & 0 deletions internal/services/groups/group_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,7 @@ func TestAccGroup_writebackUnified(t *testing.T) {
func (r GroupResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.Groups.GroupsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

group, status, err := client.Get(ctx, state.ID, odata.Query{})
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions internal/services/groups/groups_data_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ func groupsDataSource() *schema.Resource {
func groupsDataSourceRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
client := meta.(*clients.Client).Groups.GroupsClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

var groups []msgraph.Group
var expectedCount int
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,7 @@ func accessPackageAssignmentPolicyResourceDelete(ctx context.Context, d *schema.

// Wait for user object to be deleted
if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
if _, status, err := client.Get(ctx, accessPackageAssignmentPolicyId, odata.Query{}); err != nil {
if status == http.StatusNotFound {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ func TestAccAccessPackageAssignmentPolicy_update(t *testing.T) {
func (AccessPackageAssignmentPolicyResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.IdentityGovernance.AccessPackageAssignmentPolicyClient
client.BaseClient.DisableRetries = true
defer func() { client.BaseClient.DisableRetries = false }()

_, status, err := client.Get(ctx, state.ID, odata.Query{})
if err != nil {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ func accessPackageCatalogResourceDelete(ctx context.Context, d *schema.ResourceD

// Wait for object to be deleted
if err := helpers.WaitForDeletion(ctx, func(ctx context.Context) (*bool, error) {
defer func() { client.BaseClient.DisableRetries = false }()
client.BaseClient.DisableRetries = true
if _, status, err := client.Get(ctx, accessPackageCatalogId, odata.Query{}); err != nil {
if status == http.StatusNotFound {
Expand Down
Loading

0 comments on commit 3ac0b3a

Please sign in to comment.