Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 3 additions & 4 deletions src/azure-cli/azure/cli/command_modules/vm/_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -1017,15 +1017,14 @@ def load_arguments(self, _):
for scope in ['vm create', 'vmss create', 'vm identity assign', 'vmss identity assign']:
with self.argument_context(scope) as c:
arg_group = 'Managed Service Identity' if scope.split()[-1] == 'create' else None
c.argument('identity_scope', options_list=['--scope'], arg_group=arg_group, help="Scope that the system assigned identity can access")
c.argument('identity_scope', options_list=['--scope'], arg_group=arg_group,
help="Scope that the system assigned identity can access. ")
c.ignore('identity_role_id')

for scope in ['vm create', 'vmss create']:
with self.argument_context(scope) as c:
c.argument('identity_role', options_list=['--role'], arg_group='Managed Service Identity',
help='Role name or id the system assigned identity will have. '
'Please note that the default value "Contributor" will be removed in the future version 2.35.0, '
"so please specify '--role' and '--scope' at the same time when assigning a role to the managed identity")
help='Role name or id the system assigned identity will have. ')

for scope in ['vm identity assign', 'vmss identity assign']:
with self.argument_context(scope) as c:
Expand Down
39 changes: 22 additions & 17 deletions src/azure-cli/azure/cli/command_modules/vm/_validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -1210,17 +1210,32 @@ def _validate_ssh_key_helper(ssh_key_value, should_generate_ssh_keys):
return content


def _validate_vm_vmss_msi(cmd, namespace, from_set_command=False):
if from_set_command or namespace.assign_identity is not None:
def _validate_vm_vmss_msi(cmd, namespace, is_identity_assign=False):

# For the creation of VM and VMSS, "--role" and "--scope" should be passed in at the same time
# when assigning a role to the managed identity
if not is_identity_assign and namespace.assign_identity is not None:
if (namespace.identity_scope and not namespace.identity_role) or \
(not namespace.identity_scope and namespace.identity_role):
Comment on lines +1218 to +1219
Copy link
Member

Choose a reason for hiding this comment

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

For operator precedence, not > and > or, so parentheses are not necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, actually I know this. I put parentheses here just for clearer readability~

raise ArgumentUsageError(
"usage error: please specify both --role and --scope when assigning a role to the managed identity")

# For "az vm identity assign", "--scope" must be passed in when assigning a role to the managed identity
if is_identity_assign:
role_is_explicitly_specified = getattr(namespace.identity_role, 'is_default', None) is None
if not namespace.identity_scope and role_is_explicitly_specified:
raise ArgumentUsageError(
"usage error: please specify --scope when assigning a role to the managed identity")

# Assign managed identity
if is_identity_assign or namespace.assign_identity is not None:
identities = namespace.assign_identity or []
from ._vm_utils import MSI_LOCAL_ID
for i, _ in enumerate(identities):
if identities[i] != MSI_LOCAL_ID:
identities[i] = _get_resource_id(cmd.cli_ctx, identities[i], namespace.resource_group_name,
'userAssignedIdentities', 'Microsoft.ManagedIdentity')
if not namespace.identity_scope and getattr(namespace.identity_role, 'is_default', None) is None:
raise ArgumentUsageError("usage error: '--role {}' is not applicable as the '--scope' is not provided".
format(namespace.identity_role))

user_assigned_identities = [x for x in identities if x != MSI_LOCAL_ID]
if user_assigned_identities and not cmd.supported_api_version(min_api='2017-12-01'):
raise ArgumentUsageError('usage error: user assigned identity is only available under profile '
Expand All @@ -1232,19 +1247,9 @@ def _validate_vm_vmss_msi(cmd, namespace, from_set_command=False):
# keep 'identity_role' for output as logical name is more readable
setattr(namespace, 'identity_role_id', _resolve_role_id(cmd.cli_ctx, namespace.identity_role,
namespace.identity_scope))
elif namespace.identity_scope or getattr(namespace.identity_role, 'is_default', None) is None:
elif namespace.identity_scope or namespace.identity_role:
raise ArgumentUsageError('usage error: --assign-identity [--scope SCOPE] [--role ROLE]')

# For the creation of VM and VMSS, the default value "Contributor" of "--role" will be removed in the future.
# Therefore, the first step is to prompt users that parameters "--role" and "--scope" should be passed in
# at the same time to reduce the impact of breaking change
if not from_set_command and namespace.identity_scope and getattr(namespace.identity_role, 'is_default', None):
logger.warning(
"Please note that the default value of parameter '--role' will be removed in the future version 2.35.0. "
"So specify '--role' and '--scope' at the same time when assigning a role to the managed identity "
"to avoid breaking your automation script when the default value of '--role' is removed."
)


def _validate_vm_vmss_set_applications(cmd, namespace): # pylint: disable=unused-argument
if namespace.application_configuration_overrides and \
Expand Down Expand Up @@ -1770,7 +1775,7 @@ def process_disk_encryption_namespace(cmd, namespace):


def process_assign_identity_namespace(cmd, namespace):
_validate_vm_vmss_msi(cmd, namespace, from_set_command=True)
_validate_vm_vmss_msi(cmd, namespace, is_identity_assign=True)


def process_remove_identity_namespace(cmd, namespace):
Expand Down
4 changes: 2 additions & 2 deletions src/azure-cli/azure/cli/command_modules/vm/custom.py
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ def create_vm(cmd, vm_name, resource_group_name, image=None, size='Standard_DS1_
storage_account_type=None, vnet_type=None, nsg_type=None, public_ip_address_type=None, nic_type=None,
validate=False, custom_data=None, secrets=None, plan_name=None, plan_product=None, plan_publisher=None,
plan_promotion_code=None, license_type=None, assign_identity=None, identity_scope=None,
identity_role='Contributor', identity_role_id=None, application_security_groups=None, zone=None,
identity_role=None, identity_role_id=None, application_security_groups=None, zone=None,
boot_diagnostics_storage=None, ultra_ssd_enabled=None,
ephemeral_os_disk=None, ephemeral_os_disk_placement=None,
proximity_placement_group=None, dedicated_host=None, dedicated_host_group=None, aux_subscriptions=None,
Expand Down Expand Up @@ -2818,7 +2818,7 @@ def create_vmss(cmd, vmss_name, resource_group_name, image=None,
public_ip_address_type=None, storage_profile=None,
single_placement_group=None, custom_data=None, secrets=None, platform_fault_domain_count=None,
plan_name=None, plan_product=None, plan_publisher=None, plan_promotion_code=None, license_type=None,
assign_identity=None, identity_scope=None, identity_role='Contributor',
assign_identity=None, identity_scope=None, identity_role=None,
identity_role_id=None, zones=None, priority=None, eviction_policy=None,
application_security_groups=None, ultra_ssd_enabled=None,
ephemeral_os_disk=None, ephemeral_os_disk_placement=None,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,31 +288,41 @@ def test_get_next_subnet_addr_suffix(self):
@mock.patch('azure.cli.core.commands.client_factory.get_subscription_id', autospec=True)
def test_validate_msi_on_create(self, mock_get_subscription, mock_resolve_role_id):
# check throw on : az vm/vmss create --assign-identity --role reader --scope ""
from azure.cli.core.azclierror import ArgumentUsageError

np_mock = mock.MagicMock()
cmd = mock.MagicMock()
cmd.cli_ctx = DummyCli()
np_mock.assign_identity = []
np_mock.identity_scope = None
np_mock.identity_role = 'reader'
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock)
self.assertTrue("usage error: please specify both --role and --scope "
"when assigning a role to the managed identity" in str(err.exception))

with self.assertRaises(CLIError) as err:
np_mock = mock.MagicMock()
np_mock.assign_identity = []
np_mock.identity_scope = 'foo-scope'
np_mock.identity_role = None
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock)
self.assertTrue("usage error: '--role reader' is not applicable as the '--scope' is "
"not provided" in str(err.exception))
self.assertTrue("usage error: please specify both --role and --scope "
"when assigning a role to the managed identity" in str(err.exception))

# check throw on : az vm/vmss create --scope "some scope"
np_mock = mock.MagicMock()
np_mock.assign_identity = None
np_mock.identity_scope = 'foo-scope'
with self.assertRaises(CLIError) as err:
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock)
self.assertTrue('usage error: --assign-identity [--scope SCOPE] [--role ROLE]' in str(err.exception))

# check throw on : az vm/vmss create --role "reader"
np_mock = mock.MagicMock()
np_mock.assign_identity = None
np_mock.identity_role = 'reader'
with self.assertRaises(CLIError) as err:
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock)
self.assertTrue('usage error: --assign-identity [--scope SCOPE] [--role ROLE]' in str(err.exception))

Expand All @@ -336,18 +346,19 @@ def test_validate_msi_on_assign_identity_command(self, mock_resolve_role_id):
np_mock.identity_scope = ''
np_mock.identity_role = 'reader'

with self.assertRaises(CLIError) as err:
_validate_vm_vmss_msi(cmd, np_mock, from_set_command=True)
self.assertTrue("usage error: '--role reader' is not applicable as the '--scope' is set to None",
str(err.exception))
from azure.cli.core.azclierror import ArgumentUsageError
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock, is_identity_assign=True)
self.assertTrue("usage error: please specify --scope when assigning a role to the managed identity"
in str(err.exception))

# check we set right role id
np_mock = mock.MagicMock()
np_mock.identity_scope = 'foo-scope'
np_mock.identity_role = 'reader'
np_mock.assign_identity = []
mock_resolve_role_id.return_value = 'foo-role-id'
_validate_vm_vmss_msi(cmd, np_mock, from_set_command=True)
_validate_vm_vmss_msi(cmd, np_mock, is_identity_assign=True)
self.assertEqual(np_mock.identity_role_id, 'foo-role-id')
mock_resolve_role_id.assert_called_with(cmd.cli_ctx, 'reader', 'foo-scope')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,31 +287,41 @@ def test_get_next_subnet_addr_suffix(self):
@mock.patch('azure.cli.core.commands.client_factory.get_subscription_id', autospec=True)
def test_validate_msi_on_create(self, mock_get_subscription, mock_resolve_role_id):
# check throw on : az vm/vmss create --assign-identity --role reader --scope ""
from azure.cli.core.azclierror import ArgumentUsageError

np_mock = mock.MagicMock()
cmd = mock.MagicMock()
cmd.cli_ctx = DummyCli()
np_mock.assign_identity = []
np_mock.identity_scope = None
np_mock.identity_role = 'reader'
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock)
self.assertTrue("usage error: please specify both --role and --scope "
"when assigning a role to the managed identity" in str(err.exception))

with self.assertRaises(CLIError) as err:
np_mock = mock.MagicMock()
np_mock.assign_identity = []
np_mock.identity_scope = 'foo-scope'
np_mock.identity_role = None
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock)
self.assertTrue("usage error: '--role reader' is not applicable as the '--scope' is "
"not provided" in str(err.exception))
self.assertTrue("usage error: please specify both --role and --scope "
"when assigning a role to the managed identity" in str(err.exception))

# check throw on : az vm/vmss create --scope "some scope"
np_mock = mock.MagicMock()
np_mock.assign_identity = None
np_mock.identity_scope = 'foo-scope'
with self.assertRaises(CLIError) as err:
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock)
self.assertTrue('usage error: --assign-identity [--scope SCOPE] [--role ROLE]' in str(err.exception))

# check throw on : az vm/vmss create --role "reader"
np_mock = mock.MagicMock()
np_mock.assign_identity = None
np_mock.identity_role = 'reader'
with self.assertRaises(CLIError) as err:
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock)
self.assertTrue('usage error: --assign-identity [--scope SCOPE] [--role ROLE]' in str(err.exception))

Expand All @@ -335,18 +345,19 @@ def test_validate_msi_on_assign_identity_command(self, mock_resolve_role_id):
np_mock.identity_scope = ''
np_mock.identity_role = 'reader'

with self.assertRaises(CLIError) as err:
_validate_vm_vmss_msi(cmd, np_mock, from_set_command=True)
self.assertTrue("usage error: '--role reader' is not applicable as the '--scope' is set to None",
str(err.exception))
from azure.cli.core.azclierror import ArgumentUsageError
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock, is_identity_assign=True)
self.assertTrue("usage error: please specify --scope when assigning a role to the managed identity"
in str(err.exception))

# check we set right role id
np_mock = mock.MagicMock()
np_mock.identity_scope = 'foo-scope'
np_mock.identity_role = 'reader'
np_mock.assign_identity = []
mock_resolve_role_id.return_value = 'foo-role-id'
_validate_vm_vmss_msi(cmd, np_mock, from_set_command=True)
_validate_vm_vmss_msi(cmd, np_mock, is_identity_assign=True)
self.assertEqual(np_mock.identity_role_id, 'foo-role-id')
mock_resolve_role_id.assert_called_with(cmd.cli_ctx, 'reader', 'foo-scope')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,31 +287,41 @@ def test_get_next_subnet_addr_suffix(self):
@mock.patch('azure.cli.core.commands.client_factory.get_subscription_id', autospec=True)
def test_validate_msi_on_create(self, mock_get_subscription, mock_resolve_role_id):
# check throw on : az vm/vmss create --assign-identity --role reader --scope ""
from azure.cli.core.azclierror import ArgumentUsageError

np_mock = mock.MagicMock()
cmd = mock.MagicMock()
cmd.cli_ctx = DummyCli()
np_mock.assign_identity = []
np_mock.identity_scope = None
np_mock.identity_role = 'reader'
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock)
self.assertTrue("usage error: please specify both --role and --scope "
"when assigning a role to the managed identity" in str(err.exception))

with self.assertRaises(CLIError) as err:
np_mock = mock.MagicMock()
np_mock.assign_identity = []
np_mock.identity_scope = 'foo-scope'
np_mock.identity_role = None
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock)
self.assertTrue("usage error: '--role reader' is not applicable as the '--scope' is "
"not provided" in str(err.exception))
self.assertTrue("usage error: please specify both --role and --scope "
"when assigning a role to the managed identity" in str(err.exception))

# check throw on : az vm/vmss create --scope "some scope"
np_mock = mock.MagicMock()
np_mock.assign_identity = None
np_mock.identity_scope = 'foo-scope'
with self.assertRaises(CLIError) as err:
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock)
self.assertTrue('usage error: --assign-identity [--scope SCOPE] [--role ROLE]' in str(err.exception))

# check throw on : az vm/vmss create --role "reader"
np_mock = mock.MagicMock()
np_mock.assign_identity = None
np_mock.identity_role = 'reader'
with self.assertRaises(CLIError) as err:
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock)
self.assertTrue('usage error: --assign-identity [--scope SCOPE] [--role ROLE]' in str(err.exception))

Expand All @@ -335,18 +345,19 @@ def test_validate_msi_on_assign_identity_command(self, mock_resolve_role_id):
np_mock.identity_scope = ''
np_mock.identity_role = 'reader'

with self.assertRaises(CLIError) as err:
_validate_vm_vmss_msi(cmd, np_mock, from_set_command=True)
self.assertTrue("usage error: '--role reader' is not applicable as the '--scope' is set to None",
str(err.exception))
from azure.cli.core.azclierror import ArgumentUsageError
with self.assertRaises(ArgumentUsageError) as err:
_validate_vm_vmss_msi(cmd, np_mock, is_identity_assign=True)
self.assertTrue("usage error: please specify --scope when assigning a role to the managed identity"
in str(err.exception))

# check we set right role id
np_mock = mock.MagicMock()
np_mock.identity_scope = 'foo-scope'
np_mock.identity_role = 'reader'
np_mock.assign_identity = []
mock_resolve_role_id.return_value = 'foo-role-id'
_validate_vm_vmss_msi(cmd, np_mock, from_set_command=True)
_validate_vm_vmss_msi(cmd, np_mock, is_identity_assign=True)
self.assertEqual(np_mock.identity_role_id, 'foo-role-id')
mock_resolve_role_id.assert_called_with(cmd.cli_ctx, 'reader', 'foo-scope')

Expand Down
Loading