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

azurerm_role_assignment - Support scope to be /providers/Microsoft.Marketplace #18439

Closed
wants to merge 4 commits into from

Conversation

ms-zhenhua
Copy link
Contributor

@ms-zhenhua ms-zhenhua commented Sep 19, 2022

fix #18387
image

@ms-zhenhua ms-zhenhua marked this pull request as draft September 19, 2022 08:49
@ms-zhenhua ms-zhenhua marked this pull request as ready for review September 20, 2022 05:52
Copy link
Collaborator

@katbyte katbyte left a comment

Choose a reason for hiding this comment

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

@ms-zhenhua - could we update the docs & add that link and an example powershell of how to grant permissions to a SP?

@ms-zhenhua
Copy link
Contributor Author

Hi @katbyte, thank you for reviewing. I have updated the document with the related link which contains the information of how to grant permissions to different identities by Azure Portal, Azure PowerShell, Azure Cli and Rest API. Kindly help review it again. Thanks.

@ms-zhenhua ms-zhenhua requested a review from katbyte October 25, 2022 02:05
Copy link
Contributor

@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.

@ms-zhenhua I've assigned the Marketplace Admin role so you should be able to test this in TC now. I've left another comment below regarding parsing the scope, if you can rework this I'll be happy to re-review. Thanks!

website/docs/r/role_assignment.html.markdown Outdated Show resolved Hide resolved
internal/services/authorization/parse/role_assignment.go Outdated Show resolved Hide resolved
@ms-zhenhua
Copy link
Contributor Author

ms-zhenhua commented Nov 1, 2022

Hi @manicminer, thank you for reviewing. The testcase needs to assign Marketplace Admin role to a new SP. But the test SP with Marketplace Admin role does not have the permission to assign Marketplace Admin role to a new SP. To make the testcase runnable on TC, could you grant the test SP with the Owner role of private marketplace?
image

@github-actions github-actions bot added size/M and removed size/L labels Nov 1, 2022
@ms-zhenhua ms-zhenhua requested review from manicminer and removed request for katbyte November 1, 2022 06:45
@manicminer
Copy link
Contributor

@ms-zhenhua Ah, I misunderstood, this has now been assigned. Please could you add a comment to the acctest mentioning the required owner role (with docs link)? Thanks!

@github-actions github-actions bot added size/L and removed size/M labels Nov 2, 2022
@ms-zhenhua
Copy link
Contributor Author

Hi @manicminer, thank you for your update. I have added the comments to the acctest and all related testcases have passed on TC. Kindly help review it again.

@katbyte
Copy link
Collaborator

katbyte commented Nov 3, 2022

@ms-zhenhua - upon some internal discussion we've come to the conclusion that the role assignment resource should probably be broken out into separate ones for each resource type for an improved UX much like we have different resource for policy assignments.

as such for this resource we could extract it out into azurerm_role_assignment_marketplace and leave the old resource to be refactored at a later time.

WDYT?

@ms-zhenhua
Copy link
Contributor Author

Hi @katbyte, totally agree. Then I will temporarily close this PR and create a new one for the new resource azurerm_role_assignment_marketplace

@github-actions
Copy link

github-actions bot commented Dec 5, 2022

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to create role assignment at resource provider scopeof /providers/Microsoft.Marketplace
3 participants