azure_rm_roleassignment bugfix #464
Conversation
Fred-sun
added
medium_priority
Dang linter broke again. I think I've got it fixed now, hopefully this won't come up again. |
|
@Fred-sun , |
Your test case failed. Can you share your test results? Thank you very much! |
I'm not sure what you mean by share test results. I run If you're wanting a full dump of the output, it's quite a lot, and I'm not sure what private / secret information may be exposed, so I'm not comfortable copy/pasting it. Did you have a specific question? It should be too much trouble for me to sanitize the output of one or two tasks. Can you share what the failure message is that you're getting? I did make some assumptions about what permissions would be available to the testing user. In my azure environment, the service principal I'm running tests under has Owner role at the scope of the two Resource Groups set up in the cloud-config-azure.ini file, which may not hold true for your test environment. |
| loop: | ||
| - name: "{{ az_role_assignments.roleassignments[0].name }}" | ||
| scope: "{{ az_role_assignments.roleassignments[0].scope }}" | ||
| - assignee: "{{ az_role_assignments.roleassignments[0].principal_id }}" |
There was a problem hiding this comment.
@paultaiton If get role assignments info by assignee, scope and role_definition_id, It will return role assignments more than one! It will lead line 31 fail! Thank you very much!
There was a problem hiding this comment.
@paultaiton If get role assignments info by assignee, scope and role_definition_id, It will return role assignments more than one! It will lead line 31 fail! Thank you very much!
That was a bug that was legitimately caught by the test. I reordered the main 'module_exec' case in info module, but didn't add the assignee filter to the case that catches the assignment triplet. I just added it in latest commit, so test should pass now.
Thanks @Fred-sun
|
@paultaiton Thanks for your update! I will recheck it! |
Howdy @Fred-sun Thanks |
haiyuazhang
force-pushed
the
dev
branch
2 times, most recently
from
2281f46 to
8dfc8ed
Compare
|
@Fred-sun this PR has been idle for a while, can I get an update on the review status? Is there anything else I need to modify to push this along? |
I am working in this! Thank you very much! |
Fred-sun
added
ready_for_review
|
Howdy @Fred-sun |
This PR is waiting to be merged. Recently, because the developers are busy with other things, they did not merge immediately. I will ask them to help me merge as soon as possible. Thank you very much! |
|
LGTM |
|
Thanks @xuzhang3 |
* add new paramter node_labels for agent_pool * remove space Add return value for azure_rm_containerregistry idempotent test (ansible-collections#578) * Add return value for azure_rm_containerregistry idempotent test * fix sanity error: * remove require set azure_rm_roleassignment bugfix (ansible-collections#464) * pushing fixes. * whitespace * whitespace * Update aliases * add assignee filter to triplet lookup Co-authored-by: Fred-sun <[email protected]> azure_rm_aks: support system-assigned (managed) identity, (ansible-collections#514) * azure_rm_aks: make linux_profile and service_principal optional, support system-assigned (managed) identity * azure_rm_aks: adjust docs formatting Co-authored-by: Fred-sun <[email protected]> * azure_rm_aks: add a test for the minimal parameters cluster definition * azure_rm_aks: fix sanity-checks / pep8 requirements * Add instructions for tests / sanity checks Co-authored-by: Fred-sun <[email protected]> Upddate test case (ansible-collections#585)
SUMMARY
Fixes #460
The idempotence check of matching scope + role_definition_id + assignee_object_id will now correctly interpret the different forms of role_definition_id as being equivalent. This was fixed by disregarding everything else but the guid for comparison. Since guids are not supposed to be re-used in different contexts, this should be fine.
Also corrected the problems with the ansible-test integration azure_rm_roleassignment, which should now function on any resource group that is used strictly for ansible integration tests.
ISSUE TYPE
COMPONENT NAME
azure_rm_roleassignment
azure_rm_roleassignment_info
ADDITIONAL INFORMATION
The Azure API always returns role assignment results with a role_definition_id that is the fully qualified form. There are other ways of abbreviating this ID, which means you can have different forms that are equivalent. This was the cause of bug #460 .