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

Bugfix: mitigate inheritence or long race condition between resource functions when clients have retries disabled #1090

Merged
merged 1 commit into from
Apr 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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