Azure / Azure/azure-cli

Refactor `--assign-identity` and `identity assign` to common implementation

Open
#12,217 6 comments 0 reactions 2 assignees Claimed by @jiasli View on GitHub
act-platform-engineering-squad Core feature-request
Dominant language
Python
Stars
4.6k
Forks
3.5k
Avg merge
3d 2h
Merged PRs (30d)
60

Description

In the CLI today, we have multiple implementations of commands that parse `--assign-identity` and `identity assign` parameters. Since managed identities will be enabled for many Azure services, we should implement the `--assign-identity` and `identity assign` functionality once for the entire CLI and share that implementation across modules. If we don't address this now, then we'll have many more implementations in the coming months and maintenance will become that much harder with every implementation that we add.

## `--assign-identity`

The desired implementation is `az command --assign-identity [system] [user1] … [userN]`

- `--assign-identity` without any parameter value will only assign a system-assigned identity
- `--assign-identity` with `[system]` parameter value will only assign a system-assigned identity
- `--assign-identity` with `[system] [user1] [user2]` parameter value will assign BOTH system-assigned and two user-assigned principles
- `[system]` will assign a system-assigned managed identity to the resource
- `[user1] … [userN]` will assign those user-assigned managed identities to the resource
- We have custom logic around policy assignment and --assign-identity. See validate_msi method in resource/_validators.py for details.
- Today, SQL only supports system-assigned identities. We need to discuss with them if they want to support user-assigned.
- Today, functionapp only supports system-assigned identities. We need to update to support user-assigned.
- HDInsights only supports user assigned. Doesn't support system assigned - so the common method will have to validate whether or not [system] is allowed
- HDInsights supports --assign-identity [userassigned]
- HDInsights supports --storage-account-managed-identity [userassigned]

az acr task: https://github.com/Azure/azure-cli/blob/master/src/azure-cli/azure/cli/command_modules/acr/task.py#L938

```python
def _build_identities_info(cmd, identities, is_remove=False):
    IdentityProperties, UserIdentityProperties, ResourceIdentityType = cmd.get_models(
        'IdentityProperties', 'UserIdentityProperties', 'ResourceIdentityType')
    identities = identities or []
    identity_types = []
    if IDENTITY_GLOBAL_REMOVE in identities:
        return IdentityProperties(type=ResourceIdentityType.none.value)
    if not identities or IDENTITY_LOCAL_ID in identities:
        identity_types.append(ResourceIdentityType.system_assigned.value)
    external_identities = [x for x in identities if x != IDENTITY_LOCAL_ID]
    if external_identities:
        identity_types.append(ResourceIdentityType.user_assigned.value)
    identity_types = ', '.join(identity_types)
    identity = IdentityProperties(type=identity_types)
    if external_identities:
        if is_remove:
            identity.user_assigned_identities = {e: None for e in external_identities}
        else:
            identity.user_assigned_identities = {e: UserIdentityProperties() for e in external_identities}
    return identity
```

az appconfig: https://github.com/Azure/azure-cli/blob/master/src/azure-cli/azure/cli/command_modules/appconfig/custom.py#L75

```python
def __get_resource_identity(assign_identity):
    system_assigned = False
    user_assigned = {}
    for identity in assign_identity:
        if identity == SYSTEM_ASSIGNED_IDENTITY:
            system_assigned = True
        else:
            user_assigned[identity] = UserIdentity()
    if system_assigned and user_assigned:
        identity_type = SYSTEM_USER_ASSIGNED
    elif system_assigned:
        identity_type = SYSTEM_ASSIGNED
    elif user_assigned:
        identity_type = USER_ASSIGNED
    else:
        identity_type = "None"
    return ResourceIdentity(type=identity_type,
                            user_assigned_identities=user_assigned if user_assigned else None)

```

az container: https://github.com/Azure/azure-cli/blob/master/src/azure-cli/azure/cli/command_modules/container/custom.py#L231

```python
def _build_identities_info(identities):
    identities = identities or []
    identity_type = ResourceIdentityType.none
    if not identities or MSI_LOCAL_ID in identities:
        identity_type = ResourceIdentityType.system_assigned
    external_identities = [x for x in identities if x != MSI_LOCAL_ID]
    if external_identities and identity_type == ResourceIdentityType.system_assigned:
        identity_type = ResourceIdentityType.system_assigned_user_assigned
    elif external_identities:
        identity_type = ResourceIdentityType.user_assigned
    identity = ContainerGroupIdentity(type=identity_type)
    if external_identities:
        identity.user_assigned_identities = {e: {} for e in external_identities}
    return identity
```
az vm: https://github.com/Azure/azure-cli/blob/master/src/azure-cli/azure/cli/command_modules/vm/custom.py#L2448

```python
def _build_identities_info(identities):
    from ._vm_utils import MSI_LOCAL_ID
    identities = identities or []
    identity_types = []
    if not identities or MSI_LOCAL_ID in identities:
        identity_types.append('SystemAssigned')
    external_identities = [x for x in identities if x != MSI_LOCAL_ID]
    if external_identities:
        identity_types.append('UserAssigned')
    identity_types = ','.join(identity_types)
    info = {'type': identity_types}
    if external_identities:
        info['userAssignedIdentities'] = {e: {} for e in external_identities}
    return (info, identity_types, external_identities, 'SystemAssigned' in identity_types)

```
az resource: https://github.com/Azure/azure-cli/blob/master/src/azure-cli/azure/cli/command_modules/resource/custom.py#L1349

```python
def _build_identities_info(cmd, identities):
    identities = identities or []
    ResourceIdentityType = cmd.get_models('ResourceIdentityType')
    identity_type = ResourceIdentityType.none
    if not identities or MSI_LOCAL_ID in identities:
        identity_type = ResourceIdentityType.system_assigned
    ResourceIdentity = cmd.get_models('Identity')
    return ResourceIdentity(type=identity_type)

```

Hdinsight

Only supports assigning a user assigned identity, does not support system assigned identity.

```python
def build_identities_info(identities):
    from azure.mgmt.hdinsight.models import ClusterIdentity, ResourceIdentityType
    identity = None
    if identities:
        identity_type = ResourceIdentityType.user_assigned
        identity = ClusterIdentity(type=identity_type)
        identity.user_assigned_identities = {e: {} for e in identities}
    return identity

```
az sql mi create --assign-identity

Only supports system-assigned

Sql/custom.py
```python
    if assign_identity:
        kwargs['identity'] = ResourceIdentity(type=IdentityType.system_assigned.value)
```

## `identity assign | remove | list`

The interface is `az command identity assign --identities` and it follows the same convention as `--assign-identity` with `[system] [user1]…[userN]` as a parameter value.

The following support this today:
- Vm
- Appconfig
- Acr task

We'll need to add for more services:
- `functionapp`
- `appservice`
- `webapp`

We also need to support the `remove` and `list` features, which should have a shared implementation.

## MSI Validation
We have multiple implementations of validate_msi:

container\_validators.py

```python
def validate_msi(namespace):
    MSI_LOCAL_ID = '[system]'
    if namespace.assign_identity is not None:
        identities = namespace.assign_identity or []
        if not namespace.identity_scope and getattr(namespace.identity_role, 'is_default', None) is None:
            raise CLIError("usage error: '--role {}' is not applicable as the '--scope' is not provided".format(
                namespace.identity_role))
        if namespace.identity_scope:
            if identities and MSI_LOCAL_ID not in identities:
                raise CLIError("usage error: '--scope'/'--role' is only applicable when assign system identity")
    elif namespace.identity_scope or getattr(namespace.identity_role, 'is_default', None) is None:
        raise CLIError('usage error: --assign-identity [--scope SCOPE] [--role ROLE]')

```

hdinsight\_validators.py

```python
# Validate managed identity.
def validate_msi(cmd, namespace):
    namespace.assign_identity = HDInsightValidator(
        resource_type='Microsoft.ManagedIdentity/userAssignedIdentities',
        resource_name=namespace.assign_identity).validate(cmd, namespace)

# Validate managed identity to access storage account v2.
def validate_storage_msi(cmd, namespace):
    namespace.storage_account_managed_identity = HDInsightValidator(
        resource_type='Microsoft.ManagedIdentity/userAssignedIdentities',
        resource_name=namespace.storage_account_managed_identity).validate(cmd, namespace)
```
resource\_validators.py

```python
def validate_msi(namespace):
    if namespace.assign_identity is not None:
        identities = namespace.assign_identity or []
        if any(identity != MSI_LOCAL_ID for identity in identities):
            raise CLIError("usage error: 'User assigned identities are not supported "
                           "with --assign-identity and policy assignments'")
        if not namespace.identity_scope and getattr(namespace.identity_role, 'is_default', None) is None:
            raise CLIError("usage error: '--role {}' is not applicable as the '--identity-scope' is not provided"
                           .format(namespace.identity_role))
        if namespace.identity_scope:
            if identities and MSI_LOCAL_ID not in identities:
                raise CLIError(
                    "usage error: '--identity-scope'/'--role' is only applicable when assigning a system identity")
    elif namespace.identity_scope or getattr(namespace.identity_role, 'is_default', None) is None:
        raise CLIError(
            'usage error: --assign-identity [--identity-scope SCOPE] [--role ROLE]')
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.