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

Add delete function for AccessPackageResourceRoleScope #245

Merged

Conversation

alexwilcox9
Copy link
Contributor

@alexwilcox9 alexwilcox9 commented Jun 15, 2023

Hey @manicminer

Hoping this can help towards resolving hashicorp/terraform-provider-azuread#1091
Once the delete function is merged it can be referenced in this function:
https://github.com/hashicorp/terraform-provider-azuread/blob/main/internal/services/identitygovernance/access_package_resource_package_association_resource.go#L146-L150

Errors should still be expected if a user has been assigned a given resource but this will at least allow things to delete when no assignments are in place

Let me know if any tweaks are needed!

@alexwilcox9 alexwilcox9 force-pushed the AccessPackageResourceRoleScope_Delete branch from e9eaae0 to 6056da0 Compare June 15, 2023 21:20
@alexwilcox9
Copy link
Contributor Author

The second commit addresses a bug I've noticed where if a azuread_access_package_resource_package_association is deleted by a user in the console terraform errors rather than seeing the resource has gone and removing it from state

This happens because the AccessPackageResourceRoleScopeClient.Get function returns the status of the HTTP request to the accessPackages endpoint which does succeed but then as it parses the output and fails to find a matching entry it does not update that status to 404
The above results in this if statement never firing
https://github.com/hashicorp/terraform-provider-azuread/blob/main/internal/services/identitygovernance/access_package_resource_package_association_resource.go#L124-L127

If it is more appropriate to always hand back the status code from the endpoint and instead this should be handled in the Terraform code let me know and I'll role this commit back

@alexwilcox9 alexwilcox9 force-pushed the AccessPackageResourceRoleScope_Delete branch from c42881c to 23bfa7f Compare June 18, 2023 22:11
Copy link
Owner

@manicminer manicminer left a comment

Choose a reason for hiding this comment

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

Thanks @alexwilcox9, this LGTM.

Great spot on the missing resource bug. I think it's fine to fake a 404 error for now, this will be the least painful fix. We'll likely do it slightly differently in future but this works for now 👍

@manicminer manicminer added enhancement New feature or request package/msgraph labels Jul 13, 2023
@manicminer manicminer added this to the v0.62.0 milestone Jul 13, 2023
@manicminer manicminer merged commit 44feddd into manicminer:main Jul 13, 2023
manicminer added a commit that referenced this pull request Jul 13, 2023
manicminer added a commit that referenced this pull request Jul 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request package/msgraph
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants