-
Notifications
You must be signed in to change notification settings - Fork 300
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
Fix to issue: 1296 azuread_application_pre_authorized does not properly destroy #1299
Fix to issue: 1296 azuread_application_pre_authorized does not properly destroy #1299
Conversation
Ready for review! Is |
…stroy Co-authored-by: KenSpur <[email protected]>
799d815
to
1818f0a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KenSpur Thank you for submitting this PR, and apologies for the delay in reviewing.
We generally try to avoid using artificial delays as a way to combat concurrency or consistency issues. We have a locking mechanism in the provider which is intended to cover this kind of problem with parallel updates to the same underlying resource or object (i.e. an application in this case). This is present in the Create and Update functions for this resource, but it looks like I missed adding it to the Delete function.
tf.LockByName(applicationResourceName, id.ObjectId)
defer tf.UnlockByName(applicationResourceName, id.ObjectId)
Due to the tardiness of this review, I hope you don't mind but I've reworked this change using this approach, and added some checks to the acceptance test.
Thanks again for spotting this bug and working on the fix!
<Actions> <action id="6d17e7acdb2f3311576150379e22805f2f9b4aa72ff00ec136aceee45cae4b98"> <h3>Bump Terraform `azuread` provider version</h3> <details id="1d9343c012f5434ac9fe8a98135bae3667b399259be16d9b14302ea3bd424a24"> <summary>Update Terraform lock file</summary> <p>changes detected:
	"hashicorp/azuread" updated from "2.48.0" to "2.49.0" in file ".terraform.lock.hcl"</p> <details> <summary>2.49.0</summary> <pre>Changelog retrieved from:
	https://github.com/hashicorp/terraform-provider-azuread/releases/tag/v2.49.0
FEATURES:

* **New Data Source:** `azuread_group_role_management_policy` ([#1327](hashicorp/terraform-provider-azuread#1327 **New Resource:** `azuread_group_role_management_policy` ([#1327](hashicorp/terraform-provider-azuread#1327 **New Resource:** `azuread_privileged_access_group_assignment_schedule` ([#1327](hashicorp/terraform-provider-azuread#1327 **New Resource:** `azuread_privileged_access_group_eligibility_schedule` ([#1327](hashicorp/terraform-provider-azuread#1327 **New Resource:** `azuread_synchronization_job_provision_on_demand` ([#1032](https://github.com/hashicorp/terraform-provider-azuread/issues/1032))

ENHANCEMENTS:

* `data.azuread_group` - support for the `include_transitive_members` property ([#1300](hashicorp/terraform-provider-azuread#1300 `azuread_application` - relax validation for the `identifier_uris` property to allow more values ([#1351](hashicorp/terraform-provider-azuread#1351 `azuread_application_identifier_uri` - relax validation for the `identifier_uri` property to allow more values ([#1351](hashicorp/terraform-provider-azuread#1351 `azuread_group` - support the `SkipExchangeInstantOn` value for the `behaviors` property ([#1370](hashicorp/terraform-provider-azuread#1370 `azuread_user` - relax validation for the `employee_type` property to allow more values ([#1328](https://github.com/hashicorp/terraform-provider-azuread/issues/1328))

BUG FIXES:

* `azuread_application_pre_authorized` - fix a destroy-time bug that could prevent deletion of the resource ([#1299](https://github.com/hashicorp/terraform-provider-azuread/issues/1299))


</pre> </details> </details> <a href="https://infra.ci.jenkins.io/job/updatecli/job/azure/job/main/158/">Jenkins pipeline link</a> </action> </Actions> --- <table> <tr> <td width="77"> <img src="https://www.updatecli.io/images/updatecli.png" alt="Updatecli logo" width="50" height="50"> </td> <td> <p> Created automatically by <a href="https://www.updatecli.io/">Updatecli</a> </p> <details><summary>Options:</summary> <br /> <p>Most of Updatecli configuration is done via <a href="https://www.updatecli.io/docs/prologue/quick-start/">its manifest(s)</a>.</p> <ul> <li>If you close this pull request, Updatecli will automatically reopen it, the next time it runs.</li> <li>If you close this pull request and delete the base branch, Updatecli will automatically recreate it, erasing all previous commits made.</li> </ul> <p> Feel free to report any issues at <a href="https://github.com/updatecli/updatecli/issues">github.com/updatecli/updatecli</a>.<br /> If you find this tool useful, do not hesitate to star <a href="https://github.com/updatecli/updatecli/stargazers">our GitHub repository</a> as a sign of appreciation, and/or to tell us directly on our <a href="https://matrix.to/#/#Updatecli_community:gitter.im">chat</a>! </p> </details> </td> </tr> </table> Co-authored-by: Jenkins Infra Bot (updatecli) <[email protected]>
#1296