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

Azure rm role 20201018 #301

Merged

Conversation

paultaiton
Copy link
Contributor

@paultaiton paultaiton commented Oct 19, 2020

SUMMARY

Fixes multiple bugs, along with general cleanup and improvement of azure_rm_roleassignment and info modules. The additions in this PR are assuming the merging of #288 since there are shared dependencies created in azure_rm_common.py . Since I have created this branch from the branch pushed in #288, this PR strictly supersedes that one.

Besides improving detection of existing assignments for idempotency purposes, and more closely aligning the ansible module with the python SDK's requirements, there are 3 main methods for identifying a unique role assignment that have been further developed:

id: passing the fqid to get_by_id(id) in python sdk
scope and name: makes a call to get(scope, name) in the python sdk
scope, assignee_object_id and role_definition_id: fetches a list of all assignments, and filters to the (if exists,) single possible unique instance.

Fixes #145
Fixes #266
Fixes #283

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

azure_rm_roleassignment
azure_rm_roleassignment_info

ADDITIONAL INFORMATION

I have included integration tests for roleassignment modules, however I'm not certain what the test environment looks like, so I had to assume what would be OK to assign and delete based on a variable "{{ resource_group }}" that I've seen in other tests. If this needs changing let me know.

@paultaiton paultaiton mentioned this pull request Oct 19, 2020
plugins/modules/azure_rm_roleassignment.py Outdated Show resolved Hide resolved
plugins/modules/azure_rm_roleassignment.py Outdated Show resolved Hide resolved
plugins/modules/azure_rm_roleassignment_info.py Outdated Show resolved Hide resolved
plugins/modules/azure_rm_roleassignment_info.py Outdated Show resolved Hide resolved
paultaiton and others added 2 commits October 21, 2020 20:50
Co-authored-by: Fred-sun <37327967+Fred-sun@users.noreply.github.com>
@paultaiton
Copy link
Contributor Author

@Fred-sun
I have committed your suggestions except the one noted above for missing-kwoa .

@paultaiton
Copy link
Contributor Author

@Fred-sun
I think I got the missing-kwoa too. I think I deleted a lint directive that I must have overlooked as being a comment from before re-factoring that piece to use the assignment_model from azure_rm_common.

@Fred-sun
Copy link
Collaborator

@Fred-sun
I think I got the missing-kwoa too. I think I deleted a lint directive that I must have overlooked as being a comment from before re-factoring that piece to use the assignment_model from azure_rm_common.

I will recheck this! Thank you very much!

@Fred-sun Fred-sun added medium_priority Medium priority work in In trying to solve, or in working with contributors labels Oct 23, 2020
@Fred-sun Fred-sun added ready_for_review The PR has been modified and can be reviewed and merged and removed work in In trying to solve, or in working with contributors labels Nov 5, 2020
@haiyuazhang haiyuazhang merged commit 0f31447 into ansible-collections:dev Nov 6, 2020
@paultaiton paultaiton deleted the azure_rm_role-20201018 branch November 6, 2020 05:59
@paultaiton
Copy link
Contributor Author

Thank you @Fred-sun and @haiyuazhang , I know this was a large change to review, so I appreciate your effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
medium_priority Medium priority ready_for_review The PR has been modified and can be reviewed and merged
Projects
None yet
3 participants