Skip to content

{Role} Fix: az role assignment list-changelogs fails with KeyError #14461

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.

Sign up for GitHub

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

Merged
jiasli merged 7 commits into from
Sep 10, 2020
Merged

{Role} Fix: az role assignment list-changelogs fails with KeyError #14461

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0295378
Fix: `az role assignment list-changelogs` fails with KeyError
jiasli Jul 22, 2020
fe66514
non-existing
jiasli Aug 20, 2020
fe534e3
Merge branch 'dev' into list-changelogs
jiasli Aug 21, 2020
ff12d19
role_defs.get(role_resource_name)
jiasli Aug 21, 2020
1c48424
Merge branch 'dev' into list-changelogs
jiasli Sep 9, 2020
6622db8
Refine
jiasli Sep 9, 2020
ab5bbf2
Simplify code
jiasli Sep 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions src/azure-cli/azure/cli/command_modules/role/custom.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,11 @@ def list_role_assignment_change_logs(cmd, start_time=None, end_time=None): # py
result = []
worker = MultiAPIAdaptor(cmd.cli_ctx)
start_events, end_events, offline_events, client = _get_assignment_events(cmd.cli_ctx, start_time, end_time)
role_defs = {d.id: [worker.get_role_property(d, 'role_name'),
d.id.split('/')[-1]] for d in list_role_definitions(cmd)}

# Use the resource `name` of roleDefinitions as keys, instead of `id`, because `id` can be inherited.
# name: b24988ac-6180-42a0-ab88-20f7382dd24c
# id: /subscriptions/0b1f6471-1bf0-4dda-aec3-cb9272f09590/providers/Microsoft.Authorization/roleDefinitions/b24988ac-6180-42a0-ab88-20f7382dd24c # pylint: disable=line-too-long
role_defs = {d.name: worker.get_role_property(d, 'role_name') for d in list_role_definitions(cmd)}

for op_id in start_events:
e = end_events.get(op_id, None)
Expand Down Expand Up @@ -331,8 +334,12 @@ def list_role_assignment_change_logs(cmd, start_time=None, end_time=None): # py
else:
entry['scopeType'] = 'Resource'

entry['roleDefinitionId'] = role_defs[payload['roleDefinitionId']][1]
entry['roleName'] = role_defs[payload['roleDefinitionId']][0]
# Look up the resource `name`, like b24988ac-6180-42a0-ab88-20f7382dd24c
role_resource_name = payload['roleDefinitionId'].split('/')[-1]
entry['roleDefinitionId'] = role_resource_name
Copy link
Member

@jsntcy jsntcy Sep 9, 2020

Choose a reason for hiding this comment

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

Who will use entry['roleDefinitionId'] later? Now it is like b24988ac-6180-42a0-ab88-20f7382dd24c, is it expected?

Copy link
Member Author

@jiasli jiasli Sep 9, 2020

Choose a reason for hiding this comment

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

This key has the same value as the original behavior:

d.id.split('/')[-1]

I didn't change the value.

# In case the role definition has been deleted.
entry['roleName'] = role_defs.get(role_resource_name, "N/A")

result.append(entry)

# Fill in logical user/sp names as guid principal-id not readable
Expand Down