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.

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

[RBAC] BREAKING CHANGE: Fix #11883: az role assignment create: empty scope will prompt error #11983

Merged
merged 4 commits into from
Jan 30, 2020
Merged

[RBAC] BREAKING CHANGE: Fix #11883: az role assignment create: empty scope will prompt error #11983

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7e9e1c6
[RBAC] BREAKING CHANGE: Fix #11883: `az role assignment create`: empt…
arrownj Jan 29, 2020
456acef
add test for role assignment with empty scope case
arrownj Jan 29, 2020
b1ad459
udpate HISTORY.rst according to alphabetic order
arrownj Jan 30, 2020
291404e
Merge branch 'dev' into fix/scope_empty
arrownj Jan 30, 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
4 changes: 4 additions & 0 deletions src/azure-cli/HISTORY.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
Release History
===============

**RBAC**

* [BREAKING CHANGE] Fix #11883: `az role assignment create`: empty scope will prompt error
Copy link
Member

Choose a reason for hiding this comment

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

Please place this entry in the right place in the history file following alphabetic order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated.


**ACR**

* [BREAKING CHANGE] `az acr delete` will prompt
Expand Down
2 changes: 2 additions & 0 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 @@ -494,6 +494,8 @@ def _build_role_scope(resource_group_name, scope, subscription_id):
if resource_group_name:
err = 'Resource group "{}" is redundant because scope is supplied'
raise CLIError(err.format(resource_group_name))
elif scope == '':
raise CLIError('Invalid scope. Please use --help to view the valid format.')
elif resource_group_name:
scope = subscription_scope + '/resourceGroups/' + resource_group_name
else:
Expand Down
6 changes: 6 additions & 0 deletions src/azure-cli/azure/cli/command_modules/role/tests/latest/test_role.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import mock
import unittest

from knack.util import CLIError
from azure_devtools.scenario_tests import AllowLargeResponse, record_only
from azure.cli.core.profiles import ResourceType, get_sdk
from azure.cli.testsdk import ScenarioTest, LiveScenarioTest, ResourceGroupPreparer, KeyVaultPreparer
Expand Down Expand Up @@ -295,6 +296,11 @@ def test_role_assignment_e2e(self, resource_group):
self.cmd('role assignment list --assignee {upn}',
checks=self.check("length([])", 1))
self.cmd('role assignment delete --assignee {upn} --role reader')

# test role assignment on empty scope
with self.assertRaisesRegexp(CLIError, 'Invalid scope. Please use --help to view the valid format.'):
Copy link
Member

Choose a reason for hiding this comment

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

Usually we just use some phrases in the error message, like "Invalid scope", to have more flexibility of the error message in the future. But this is totally fine to be more strict on the error message. 😉

self.cmd('role assignment create --assignee {upn} --scope "" --role reader')
self.cmd('role assignment delete --assignee {upn} --scope "" --role reader')
finally:
self.cmd('ad user delete --upn-or-object-id {upn}')

Expand Down