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

added missing built-in roles. open issues 670 and 671 will be fixed. #755

Closed
wants to merge 1 commit into from
Closed

Conversation

aantony2
Copy link
Contributor

Working with Arun Chandrasekhar from Microsoft on this issue. We are trying use Terraform modules for Azure Role Definitions and Assignments. Looks like current code supports only 4 built-in roles. Hence added missing built-in roles and VirtualMachineOperator had an incorrect GUID. Updated that GUID as well.

Thanks,
Agnel

@paultyng
Copy link
Contributor

Where did the data come from? Is it somewhere we could curl? Maybe write a little script to generate this?

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @agnelantony2

Thanks for this PR - in order for us to be able to ship this as part of the next release (which we'll be releasing shortly), I'm going to push a couple of commits to update the tests to use this new UUID/GUID; but this otherwise LGTM :)

Thanks!

@tombuildsstuff
Copy link
Contributor

hey @agnelantony2

As mentioned above I've added a commit to fix the tests - however unfortunately I'm unable to push to your fork (so that we could get this into the release which is happening shortly).

I hope you don't mind, but I've re-opened this PR as #762 which includes your original commit and the follow up that I've done - but this will allow us to merge this in as part of the current release. As such I'm going to close this PR for the moment - but I'd like to thank you for this contribution - and it's worth noting that the original commit is still authored by you :)

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants