diff --git a/src/azure-cli/HISTORY.rst b/src/azure-cli/HISTORY.rst index b448e89b0a8..a593ae91fe6 100644 --- a/src/azure-cli/HISTORY.rst +++ b/src/azure-cli/HISTORY.rst @@ -3,6 +3,17 @@ Release History =============== +Upcoming +++++++++ + +**ACR** + +* `az acr connected-registry create/update`: Add ``--auth-type`` (``SyncToken`` | ``ManagedIdentity``) and ``--identity`` to support user-assigned managed identity authentication between a connected registry and its parent. +* `az acr connected-registry update`: Support migrating between ``SyncToken`` and ``ManagedIdentity`` auth modes. Registry must be in ``Offline`` state; same-mode credential rotation is not supported. +* `az acr connected-registry get-settings`: Emit a ``ManagedIdentity``-flavored connection string (``ACR_MANAGED_IDENTITY_CLIENT_ID``, ``ACR_MANAGED_IDENTITY_RESOURCE_ID``) when the target is configured with ``ManagedIdentity`` auth. +* `az acr connected-registry permissions`: Reject with a clear error when invoked against a ``ManagedIdentity``-mode connected registry (sync-token scope map is not applicable in that mode). +* Bump ``azure-mgmt-containerregistry`` to ``15.1.0b3`` for the preview api-version that exposes the connected-registry managed-identity contract. + 2.89.1 ++++++ diff --git a/src/azure-cli/azure/cli/command_modules/acr/_help.py b/src/azure-cli/azure/cli/command_modules/acr/_help.py index 21ef536511c..4521552b2bc 100644 --- a/src/azure-cli/azure/cli/command_modules/acr/_help.py +++ b/src/azure-cli/azure/cli/command_modules/acr/_help.py @@ -1655,6 +1655,11 @@ az acr connected-registry create -r mycloudregistry -n myreadonlyacr -p myconnectedregistry \\ --repository "app/mycomponent" -m ReadOnly -s "0 12 * * *" -w PT4H \\ --client-tokens myTokenName1 myTokenName2 + - name: Create a connected registry that authenticates with its parent using a user-assigned managed identity. + text: | + az acr connected-registry create --registry mycloudregistry --name myconnectedregistry \\ + --auth-type ManagedIdentity \\ + --identity /subscriptions//resourceGroups//providers/Microsoft.ManagedIdentity/userAssignedIdentities/ """ helps['acr connected-registry delete'] = """ @@ -1724,6 +1729,15 @@ text: | az acr connected-registry update --registry mycloudregistry --name myreadonlyacr \\ --sync-schedule "0 12 * * *" --sync-window PT4H + - name: Migrate an offline connected registry from SyncToken to ManagedIdentity authentication. + text: | + az acr connected-registry update --registry mycloudregistry --name myconnectedregistry \\ + --auth-type ManagedIdentity \\ + --identity /subscriptions//resourceGroups//providers/Microsoft.ManagedIdentity/userAssignedIdentities/ + - name: Migrate an offline connected registry from ManagedIdentity back to SyncToken authentication. + text: | + az acr connected-registry update --registry mycloudregistry --name myconnectedregistry \\ + --auth-type SyncToken --sync-token mySyncTokenName """ helps['acr connected-registry get-settings'] = """ @@ -1740,7 +1754,7 @@ helps['acr connected-registry permissions'] = """ type: group -short-summary: Manage the repository permissions accross multiple connected registries. Please see https://aka.ms/acr/connected-registry for more information. +short-summary: Manage the repository permissions accross multiple connected registries. Only supported for connected registries configured with SyncToken authentication (output is derived from the sync-token scope map). Please see https://aka.ms/acr/connected-registry for more information. """ helps['acr connected-registry permissions update'] = """ diff --git a/src/azure-cli/azure/cli/command_modules/acr/_params.py b/src/azure-cli/azure/cli/command_modules/acr/_params.py index b21117b5ee5..67dfd2e764a 100644 --- a/src/azure-cli/azure/cli/command_modules/acr/_params.py +++ b/src/azure-cli/azure/cli/command_modules/acr/_params.py @@ -583,6 +583,8 @@ def load_arguments(self, _): # pylint: disable=too-many-statements help='Indicate whether garbage collection is enabled. It is enabled by default.', arg_type=get_three_state_flag(), required=False, default="true") c.argument('garbage_collection_schedule', options_list=['--gc-schedule'], help='Used to determine garbage collection schedule. Uses cron expression to determine the schedule. If not specified, garbage collection is set to run once a day.', required=False, default="0 0 * * *") + c.argument('identity', options_list=['--identity'], help='Resource ID of a user-assigned managed identity to authenticate the connected registry with its parent. Required when --auth-type is ManagedIdentity.') + c.argument('auth_type', arg_type=get_enum_type(['SyncToken', 'ManagedIdentity']), options_list=['--auth-type'], help='Authentication type used by the connected registry to sync with its parent. Defaults to SyncToken.') with self.argument_context('acr connected-registry update') as c: c.argument('log_level', help='Set the log level for logging on the instance. Accepted log levels are Debug, Information, Warning, Error, and None.') @@ -600,6 +602,9 @@ def load_arguments(self, _): # pylint: disable=too-many-statements c.argument('garbage_collection_enabled', options_list=['--gc-enabled'], help='Indicate whether garbage collection is enabled. It is enabled by default.', arg_type=get_three_state_flag()) c.argument('garbage_collection_schedule', options_list=['--gc-schedule'], help='Used to determine garbage collection schedule. Uses cron expression to determine the schedule. If not specified, garbage collection is set to run once a day.') + c.argument('identity', options_list=['--identity'], help='Resource ID of a user-assigned managed identity. Required when migrating --auth-type to ManagedIdentity.') + c.argument('auth_type', arg_type=get_enum_type(['SyncToken', 'ManagedIdentity']), options_list=['--auth-type'], help='Target authentication type. Use to migrate between SyncToken and ManagedIdentity. Same-mode credential rotation is not supported and connected registry must be in Offline state.') + c.argument('sync_token_name', options_list=['--sync-token'], help='Existing sync token name. Required when migrating --auth-type back to SyncToken.') with self.argument_context('acr connected-registry permissions') as c: c.argument('add_repos', options_list=['--add'], nargs='*', help='repository permissions to be added to the targeted connected registry and it\'s ancestors sync scope maps. Use the format "--add [REPO1 REPO2 ...]" per flag. ' + repo_valid_actions) diff --git a/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py b/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py index 24cc66efa62..608c456438c 100644 --- a/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py +++ b/src/azure-cli/azure/cli/command_modules/acr/connected_registry.py @@ -11,6 +11,24 @@ from azure.cli.core.commands import LongRunningOperation from azure.cli.core.commands.client_factory import get_subscription_id from azure.cli.core.util import user_confirmation +# Preview azure-mgmt-containerregistry 15.1.0b3 uses a flat model namespace (no api-version +# subpackages), so `cmd.get_models` cannot resolve these types. Import them directly. +from azure.mgmt.containerregistry.models import ( + AuthType, + ConnectedRegistry, + ConnectedRegistryUpdateParameters, + ConnectionState, + GarbageCollectionProperties, + LoggingProperties, + ManagedServiceIdentity, + ManagedServiceIdentityType, + ParentProperties, + ScopeMapUpdateParameters, + SyncProperties, + SyncUpdateProperties, + Token, + UserAssignedIdentity, +) from ._client_factory import cf_acr_tokens, cf_acr_scope_maps, cf_acr_registries from ._utils import ( build_token_id, @@ -41,10 +59,47 @@ class ConnectedRegistryModes(Enum): REPOSITORY = "repositories/" GATEWAY = "gateway/" +# Auth-type / identity / connection-state string constants sourced from the SDK enums so a +# rename or removal in the SDK becomes an import-time AttributeError rather than a silent +# string-comparison mismatch. Values on the wire remain plain strings; comparisons throughout +# this module are string-to-string (server-returned enums are coerced via ``getattr(.., 'value', ..)``). +AUTH_TYPE_SYNC_TOKEN = AuthType.SYNC_TOKEN.value +AUTH_TYPE_MANAGED_IDENTITY = AuthType.MANAGED_IDENTITY.value +MSI_TYPE_USER_ASSIGNED = ManagedServiceIdentityType.USER_ASSIGNED.value +CONNECTION_STATE_OFFLINE = ConnectionState.OFFLINE.value + + +def _get_current_auth_type(connected_registry): + """Return the current auth type ('SyncToken' or 'ManagedIdentity') of a connected registry. + + Falls back to 'SyncToken' when the server has not populated auth_type (older resources). + Coerces the SDK ``AuthType`` enum (if present) to its string value so downstream string + comparisons and error messages behave predictably. + """ + identity = getattr(connected_registry, 'identity', None) + if identity is not None and identity.type and MSI_TYPE_USER_ASSIGNED in str(identity.type): + return AUTH_TYPE_MANAGED_IDENTITY + try: + auth_type = connected_registry.parent.sync_properties.auth_type + except AttributeError: + auth_type = None + # The SDK deserializes auth_type as an ``AuthType`` enum; use ``.value`` when available. + auth_type = getattr(auth_type, 'value', auth_type) + return auth_type or AUTH_TYPE_SYNC_TOKEN + + +def _build_user_assigned_identity(identity_resource_id): + """Wrap a single user-assigned identity resource ID in a ManagedServiceIdentity.""" + return ManagedServiceIdentity( + type=MSI_TYPE_USER_ASSIGNED, + user_assigned_identities={identity_resource_id: UserAssignedIdentity()} + ) + + logger = get_logger(__name__) -def acr_connected_registry_create(cmd, # pylint: disable=too-many-locals, too-many-statements +def acr_connected_registry_create(cmd, # pylint: disable=too-many-locals, too-many-statements, too-many-branches client, registry_name, connected_registry_name, @@ -62,10 +117,29 @@ def acr_connected_registry_create(cmd, # pylint: disable=too-many-locals, too-m notifications=None, garbage_collection_enabled=None, garbage_collection_schedule=None, + identity=None, + auth_type=None, yes=False): - if bool(sync_token_name) == bool(repositories): - raise CLIError("argument error: either --sync-token or --repository must be provided, but not both.") + is_managed_identity = auth_type == AUTH_TYPE_MANAGED_IDENTITY + if is_managed_identity: + if not identity: + raise ArgumentUsageError( + "argument error: --identity is required " + "when --auth-type ManagedIdentity." + ) + if sync_token_name or repositories: + raise ArgumentUsageError( + "argument error: --sync-token and --repository are not applicable when " + "--auth-type ManagedIdentity." + ) + else: + if identity: + raise ArgumentUsageError( + "argument error: --identity is only applicable with --auth-type ManagedIdentity." + ) + if bool(sync_token_name) == bool(repositories): + raise CLIError("argument error: either --sync-token or --repository must be provided, but not both.") # Check needed since the sync token gateway actions must be at least 5 characters long. if len(connected_registry_name) < 5: raise InvalidArgumentValueError("argument error: Connected registry name must be at least 5 characters long.") @@ -103,7 +177,9 @@ def acr_connected_registry_create(cmd, # pylint: disable=too-many-locals, too-m _update_ancestor_permissions(cmd, family_tree, resource_group_name, registry_name, parent.id, connected_registry_name, repositories, mode, False) - if sync_token_name: + if is_managed_identity: + sync_token_id = None + elif sync_token_name: sync_token_id = build_token_id(subscription_id, resource_group_name, registry_name, sync_token_name) else: sync_token_id = _create_sync_token(cmd, resource_group_name, registry_name, @@ -117,12 +193,7 @@ def acr_connected_registry_create(cmd, # pylint: disable=too-many-locals, too-m notifications_set = set(notifications) \ if notifications else set() - ConnectedRegistry, LoggingProperties, SyncProperties, \ - ParentProperties, GarbageCollectionProperties = cmd.get_models( - 'ConnectedRegistry', 'LoggingProperties', 'SyncProperties', - 'ParentProperties', 'GarbageCollectionProperties') connected_registry_create_parameters = ConnectedRegistry( - provisioning_state=None, mode=mode, parent=ParentProperties( id=parent.id if parent else None, @@ -130,7 +201,8 @@ def acr_connected_registry_create(cmd, # pylint: disable=too-many-locals, too-m token_id=sync_token_id, schedule=sync_schedule, message_ttl=sync_message_ttl, - sync_window=sync_window + sync_window=sync_window, + auth_type=AUTH_TYPE_MANAGED_IDENTITY if is_managed_identity else AUTH_TYPE_SYNC_TOKEN, ) ), client_token_ids=client_token_list, @@ -142,7 +214,8 @@ def acr_connected_registry_create(cmd, # pylint: disable=too-many-locals, too-m enabled=garbage_collection_enabled, schedule=garbage_collection_schedule ), - notifications_list=list(notifications_set) if notifications_set else None + notifications_list=list(notifications_set) if notifications_set else None, + identity=_build_user_assigned_identity(identity) if is_managed_identity else None, ) try: @@ -154,7 +227,7 @@ def acr_connected_registry_create(cmd, # pylint: disable=too-many-locals, too-m raise CLIError(e) -def acr_connected_registry_update(cmd, # pylint: disable=too-many-locals, too-many-statements +def acr_connected_registry_update(cmd, # pylint: disable=too-many-locals, too-many-statements, too-many-branches client, registry_name, connected_registry_name, @@ -169,13 +242,74 @@ def acr_connected_registry_update(cmd, # pylint: disable=too-many-locals, too-m add_notifications=None, remove_notifications=None, garbage_collection_enabled=None, - garbage_collection_schedule=None): + garbage_collection_schedule=None, + identity=None, + auth_type=None, + sync_token_name=None): _, resource_group_name = validate_managed_registry( cmd, registry_name, resource_group_name) subscription_id = get_subscription_id(cmd.cli_ctx) current_connected_registry = acr_connected_registry_show( cmd, client, connected_registry_name, registry_name, resource_group_name) + # --- Auth migration validation (SyncToken <-> ManagedIdentity, no same-mode rotation) --- + current_auth_type = _get_current_auth_type(current_connected_registry) + identity_update = None + sync_auth_type_update = None + sync_token_id_update = None + + if auth_type or identity: + if not auth_type: + raise ArgumentUsageError( + "argument error: --auth-type is required when --identity is provided during update." + ) + if auth_type == current_auth_type: + raise ArgumentUsageError( + "argument error: connected registry is already using '{}' authentication. " + "Same-mode credential rotation is not supported.".format(current_auth_type) + ) + current_state = getattr(current_connected_registry, 'connection_state', None) + if current_state != CONNECTION_STATE_OFFLINE: + raise ArgumentUsageError( + "argument error: connected registry must be in '{}' state to migrate authentication mode. " + "Current state is '{}'. Deactivate it first with " + "'az acr connected-registry deactivate'.".format(CONNECTION_STATE_OFFLINE, current_state) + ) + if auth_type == AUTH_TYPE_MANAGED_IDENTITY: + if not identity: + raise ArgumentUsageError( + "argument error: --identity is required " + "when migrating to --auth-type ManagedIdentity." + ) + if sync_token_name: + raise ArgumentUsageError( + "argument error: --sync-token is not applicable when migrating to ManagedIdentity." + ) + identity_update = _build_user_assigned_identity(identity) + sync_auth_type_update = AUTH_TYPE_MANAGED_IDENTITY + else: # migrating to SyncToken + if identity: + raise ArgumentUsageError( + "argument error: --identity is only applicable with --auth-type ManagedIdentity." + ) + if not sync_token_name: + raise ArgumentUsageError( + "argument error: --sync-token is required when migrating to " + "--auth-type SyncToken." + ) + sync_token_id_update = build_token_id( + subscription_id, resource_group_name, registry_name, sync_token_name) + sync_auth_type_update = AUTH_TYPE_SYNC_TOKEN + # Do NOT send `identity` in the PATCH body when migrating to SyncToken. + # The RP rejects any non-null `identity` object (even {type: "None"}) combined + # with authType=SyncToken as ConnectedRegistryConflictingAuthInput. The server + # clears the resource-level identity itself when authType flips to SyncToken. + identity_update = None + elif sync_token_name: + raise ArgumentUsageError( + "argument error: --sync-token is only applicable when migrating auth-type." + ) + # Add or remove from the current client token id list if add_client_token_list is not None: for i, client_token_name in enumerate(add_client_token_list): @@ -225,15 +359,13 @@ def acr_connected_registry_update(cmd, # pylint: disable=too-many-locals, too-m notifications_list = list(notifications_set) if notifications_set != current_notifications_set else None - ConnectedRegistryUpdateParameters, SyncUpdateProperties, \ - LoggingProperties, GarbageCollectionProperties = cmd.get_models( - 'ConnectedRegistryUpdateParameters', 'SyncUpdateProperties', - 'LoggingProperties', 'GarbageCollectionProperties') connected_registry_update_parameters = ConnectedRegistryUpdateParameters( sync_properties=SyncUpdateProperties( schedule=sync_schedule, message_ttl=sync_message_ttl, - sync_window=sync_window + sync_window=sync_window, + token_id=sync_token_id_update, + auth_type=sync_auth_type_update, ), logging=LoggingProperties( log_level=log_level, @@ -244,7 +376,8 @@ def acr_connected_registry_update(cmd, # pylint: disable=too-many-locals, too-m schedule=garbage_collection_schedule ), client_token_ids=client_token_list, - notifications_list=notifications_list + notifications_list=notifications_list, + identity=identity_update, ) try: @@ -274,6 +407,14 @@ def acr_connected_registry_delete(cmd, connected_registry = acr_connected_registry_show( cmd, client, connected_registry_name, registry_name, resource_group_name) result = client.begin_delete(resource_group_name, registry_name, connected_registry_name).result() + # ManagedIdentity-mode connected registries have no sync token or scope map, so there is + # nothing to clean up. --cleanup, if passed, is a no-op in this mode. + if _get_current_auth_type(connected_registry) == AUTH_TYPE_MANAGED_IDENTITY: + if cleanup: + logger.warning( + "'--cleanup' has no effect on ManagedIdentity-mode connected registry '%s' " + "(no sync token or scope map exists).", connected_registry_name) + return result sync_token = get_token_from_id(cmd, connected_registry.parent.sync_properties.token_id) sync_token_name = sync_token.name sync_scope_map_name = sync_token.scope_map_id.split('/scopeMaps/')[1] @@ -402,7 +543,6 @@ def _create_sync_token(cmd, sync_token_name = connected_registry_name logger.warning("If sync token '%s' already exists, it properties will be overwritten", sync_token_name) - Token = cmd.get_models('Token') poller = token_client.begin_create( resource_group_name, registry_name, @@ -423,19 +563,19 @@ def _get_family_tree(connected_registry_list, target_connected_registry_name): family_tree = {} targetConnectedRegistry = None # Populate the dictionary - for ConnectedRegistry in connected_registry_list: - family_tree[ConnectedRegistry.id] = { - "connectedRegistry": ConnectedRegistry, + for cr in connected_registry_list: + family_tree[cr.id] = { + "connectedRegistry": cr, "children": [] } - if ConnectedRegistry.name == target_connected_registry_name: - targetConnectedRegistry = ConnectedRegistry + if cr.name == target_connected_registry_name: + targetConnectedRegistry = cr # Populate Children dependencies - for ConnectedRegistry in connected_registry_list: - parent_id = ConnectedRegistry.parent.id + for cr in connected_registry_list: + parent_id = cr.parent.id if parent_id and not parent_id.isspace(): - family_tree[parent_id]["children"].append(ConnectedRegistry.id) + family_tree[parent_id]["children"].append(cr.id) return family_tree, targetConnectedRegistry @@ -472,7 +612,7 @@ def acr_connected_registry_install_renew_credentials(cmd, '1', yes, resource_group_name) -def acr_connected_registry_get_settings(cmd, +def acr_connected_registry_get_settings(cmd, # pylint: disable=too-many-locals client, connected_registry_name, registry_name, @@ -485,6 +625,53 @@ def acr_connected_registry_get_settings(cmd, connected_registry = acr_connected_registry_show( cmd, client, connected_registry_name, registry_name, resource_group_name) + # --- Managed Identity mode: no sync-token, return MI-flavored connection string. --- + if _get_current_auth_type(connected_registry) == AUTH_TYPE_MANAGED_IDENTITY: + if generate_password: + raise ArgumentUsageError( + "argument error: --generate-password is not applicable for a connected registry " + "configured with ManagedIdentity authentication." + ) + identity = getattr(connected_registry, 'identity', None) + user_assigned = identity.user_assigned_identities if identity else None + if not user_assigned: + raise CLIError( + "Connected registry '{}' is in ManagedIdentity mode but no user-assigned identity is " + "attached.".format(connected_registry_name)) + # Spec ยง3.3: exactly one user-assigned identity is expected. + msi_resource_id, msi = next(iter(user_assigned.items())) + client_id = getattr(msi, 'client_id', None) + if not client_id: + raise CLIError( + "Client ID for user-assigned identity '{}' is not populated by the service yet.".format( + msi_resource_id)) + parent_gateway_endpoint = connected_registry.parent.sync_properties.gateway_endpoint + if not parent_gateway_endpoint: + parent_gateway_endpoint = "" + parent_id = connected_registry.parent.id + if parent_id: + parent_endpoint_protocol = parent_protocol + else: + if parent_protocol != "https": + logger.warning("Parent endpoint protocol must be 'https' when parent is a cloud registry.") + parent_endpoint_protocol = "https" + connection_string = ( + "ConnectedRegistryName=%s;" % connected_registry_name + + "ManagedIdentityClientId=%s;" % client_id + + "ParentGatewayEndpoint=%s;" % parent_gateway_endpoint + + "ParentEndpointProtocol=%s" % parent_endpoint_protocol + ) + connected_registry_login_server = "" + return { + "ACR_MANAGED_IDENTITY_CLIENT_ID": client_id, + "ACR_MANAGED_IDENTITY_RESOURCE_ID": msi_resource_id, + "ACR_REGISTRY_CERTIFICATE_VOLUME": "/var/acr/certs", + "ACR_REGISTRY_DATA_VOLUME": "/var/acr/data", + "ACR_REGISTRY_CONNECTION_STRING": connection_string, + "ACR_REGISTRY_LOGIN_SERVER": connected_registry_login_server, + } + sync_token_name = connected_registry.parent.sync_properties.token_id.split('/tokens/')[1] if generate_password: user_confirmation("Are you sure you want to generate a new sync token '{}' password{}?".format( @@ -594,7 +781,6 @@ def _update_repo_permissions(cmd, current_actions = list(final_actions_set) logger.warning(msg) - ScopeMapUpdateParameters = cmd.get_models('ScopeMapUpdateParameters') scope_map_update_parameters = ScopeMapUpdateParameters( description=description, actions=current_actions @@ -624,6 +810,12 @@ def acr_connected_registry_permissions_show(cmd, cmd, registry_name, resource_group_name) connected_registry = acr_connected_registry_show( cmd, client, connected_registry_name, registry_name, resource_group_name) + if _get_current_auth_type(connected_registry) == AUTH_TYPE_MANAGED_IDENTITY: + raise ArgumentUsageError( + "'permissions show' is only supported for connected registries configured with SyncToken " + "authentication. Output is derived from the sync-token scope map, which is not present in " + "ManagedIdentity mode." + ) sync_token = get_token_from_id(cmd, connected_registry.parent.sync_properties.token_id) return get_scope_map_from_id(cmd, sync_token.scope_map_id) @@ -653,6 +845,12 @@ def acr_connected_registry_permissions_update(cmd, family_tree, target_connected_registry = _get_family_tree(connected_registry_list, connected_registry_name) if target_connected_registry is None: raise CLIError("Connected registry '{}' doesn't exist.".format(connected_registry_name)) + if _get_current_auth_type(target_connected_registry) == AUTH_TYPE_MANAGED_IDENTITY: + raise ArgumentUsageError( + "'permissions update' is only supported for connected registries configured with SyncToken " + "authentication. Repository permissions are managed via the sync-token scope map, which is " + "not present in ManagedIdentity mode." + ) # remove repo permissions from connected registry descendants. remove_actions = REPO_SCOPES_BY_MODE[ConnectedRegistryModes.READWRITE.value] diff --git a/src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_connected_registry_mi_unit.py b/src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_connected_registry_mi_unit.py new file mode 100644 index 00000000000..138ea9837cf --- /dev/null +++ b/src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_connected_registry_mi_unit.py @@ -0,0 +1,444 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +"""Unit tests for the managed-identity code paths in +``azure.cli.command_modules.acr.connected_registry``. + +These tests deliberately import SDK models so a future flat-namespace reshape breaks the +import surface loudly (the recorded scenario in test_acr_connectedregistry_commands.py +stays SDK-shape-proof). +""" + +import unittest +from unittest import mock + +from azure.cli.core.azclierror import ArgumentUsageError +from knack.util import CLIError + +from azure.mgmt.containerregistry.models import ( + AuthType, + ConnectionState, + ManagedServiceIdentity, + ManagedServiceIdentityType, + UserAssignedIdentity, +) + +from azure.cli.command_modules.acr.connected_registry import ( + _build_user_assigned_identity, + _get_current_auth_type, + acr_connected_registry_create, + acr_connected_registry_delete, + acr_connected_registry_get_settings, + acr_connected_registry_permissions_show, + acr_connected_registry_permissions_update, + acr_connected_registry_update, + AUTH_TYPE_MANAGED_IDENTITY, + AUTH_TYPE_SYNC_TOKEN, + CONNECTION_STATE_OFFLINE, + MSI_TYPE_USER_ASSIGNED, +) + + +TEST_SUB = '00000000-0000-0000-0000-000000000001' +TEST_RG = 'rg' +TEST_REGISTRY = 'testreg' +TEST_CR = 'testcr123' +TEST_MSI_ID = ( + '/subscriptions/{}/resourceGroups/{}/providers/Microsoft.ManagedIdentity/' + 'userAssignedIdentities/msi1'.format(TEST_SUB, TEST_RG) +) +TEST_MSI_ID2 = TEST_MSI_ID.replace('msi1', 'msi2') + + +def _make_cmd(): + cmd = mock.MagicMock() + cmd.cli_ctx = mock.MagicMock() + return cmd + + +def _fake_cr(auth_type=None, has_identity=False, connection_state=None, + token_id=None, gateway_endpoint='parent.example.com', client_id='cid-1'): + """Build a fake connected-registry return object shaped like the SDK model.""" + cr = mock.MagicMock() + cr.name = TEST_CR + cr.mode = 'ReadOnly' + cr.connection_state = connection_state + cr.client_token_ids = None + cr.notifications_list = None + cr.parent = mock.MagicMock() + cr.parent.id = None + cr.parent.sync_properties = mock.MagicMock() + cr.parent.sync_properties.auth_type = auth_type + cr.parent.sync_properties.token_id = token_id + cr.parent.sync_properties.gateway_endpoint = gateway_endpoint + if has_identity: + cr.identity = mock.MagicMock() + cr.identity.type = MSI_TYPE_USER_ASSIGNED + msi = mock.MagicMock() + msi.client_id = client_id + cr.identity.user_assigned_identities = {TEST_MSI_ID: msi} + else: + cr.identity = None + return cr + + +# --------------------------------------------------------------------------- +# Constants +# --------------------------------------------------------------------------- + + +class TestConstantsSourcedFromSDK(unittest.TestCase): + """The module constants must match the SDK enum values verbatim so a rename + fails at import time rather than as a silent runtime string mismatch.""" + + def test_auth_type_constants(self): + self.assertEqual(AUTH_TYPE_SYNC_TOKEN, AuthType.SYNC_TOKEN.value) + self.assertEqual(AUTH_TYPE_MANAGED_IDENTITY, AuthType.MANAGED_IDENTITY.value) + + def test_msi_type_constant(self): + self.assertEqual(MSI_TYPE_USER_ASSIGNED, ManagedServiceIdentityType.USER_ASSIGNED.value) + + def test_connection_state_constant(self): + self.assertEqual(CONNECTION_STATE_OFFLINE, ConnectionState.OFFLINE.value) + + +# --------------------------------------------------------------------------- +# Helper: _get_current_auth_type +# --------------------------------------------------------------------------- + + +class TestGetCurrentAuthType(unittest.TestCase): + + def test_identity_attached_returns_managed_identity(self): + cr = _fake_cr(auth_type=None, has_identity=True) + self.assertEqual(_get_current_auth_type(cr), AUTH_TYPE_MANAGED_IDENTITY) + + def test_sync_token_auth_type_no_identity(self): + cr = _fake_cr(auth_type=AUTH_TYPE_SYNC_TOKEN, has_identity=False) + self.assertEqual(_get_current_auth_type(cr), AUTH_TYPE_SYNC_TOKEN) + + def test_missing_auth_type_defaults_to_sync_token(self): + cr = _fake_cr(auth_type=None, has_identity=False) + self.assertEqual(_get_current_auth_type(cr), AUTH_TYPE_SYNC_TOKEN) + + def test_enum_valued_auth_type_is_coerced_to_string(self): + cr = _fake_cr(auth_type=AuthType.MANAGED_IDENTITY, has_identity=False) + result = _get_current_auth_type(cr) + self.assertEqual(result, AUTH_TYPE_MANAGED_IDENTITY) + self.assertIsInstance(result, str) + self.assertNotIn('AuthType.', result) + + +# --------------------------------------------------------------------------- +# Helper: _build_user_assigned_identity +# --------------------------------------------------------------------------- + + +class TestBuildUserAssignedIdentity(unittest.TestCase): + + def test_shape(self): + identity = _build_user_assigned_identity(TEST_MSI_ID) + self.assertIsInstance(identity, ManagedServiceIdentity) + self.assertEqual(identity.type, MSI_TYPE_USER_ASSIGNED) + self.assertIn(TEST_MSI_ID, identity.user_assigned_identities) + self.assertIsInstance( + identity.user_assigned_identities[TEST_MSI_ID], UserAssignedIdentity) + + +# --------------------------------------------------------------------------- +# create: client-side argument validation +# --------------------------------------------------------------------------- + + +class TestConnectedRegistryCreateValidation(unittest.TestCase): + + def _create(self, **overrides): + kwargs = dict( + cmd=_make_cmd(), + client=mock.MagicMock(), + registry_name=TEST_REGISTRY, + connected_registry_name=TEST_CR, + mode='ReadOnly', + ) + kwargs.update(overrides) + return acr_connected_registry_create(**kwargs) + + def test_mi_requires_identity(self): + with self.assertRaises(ArgumentUsageError): + self._create(auth_type=AUTH_TYPE_MANAGED_IDENTITY) + + def test_mi_rejects_sync_token(self): + with self.assertRaises(ArgumentUsageError): + self._create(auth_type=AUTH_TYPE_MANAGED_IDENTITY, + identity=TEST_MSI_ID, sync_token_name='tok') + + def test_mi_rejects_repository(self): + with self.assertRaises(ArgumentUsageError): + self._create(auth_type=AUTH_TYPE_MANAGED_IDENTITY, + identity=TEST_MSI_ID, repositories=['r1']) + + def test_sync_token_rejects_identity(self): + with self.assertRaises(ArgumentUsageError): + self._create(auth_type=AUTH_TYPE_SYNC_TOKEN, identity=TEST_MSI_ID, + sync_token_name='tok') + + def test_sync_token_requires_exactly_one_of_token_or_repos(self): + with self.assertRaises(CLIError): + # neither + self._create(auth_type=AUTH_TYPE_SYNC_TOKEN) + with self.assertRaises(CLIError): + # both + self._create(auth_type=AUTH_TYPE_SYNC_TOKEN, + sync_token_name='tok', repositories=['r1']) + + def test_short_name_rejected(self): + with self.assertRaises(Exception): # InvalidArgumentValueError + self._create(auth_type=AUTH_TYPE_MANAGED_IDENTITY, + identity=TEST_MSI_ID, + connected_registry_name='abc') # < 5 chars + + +# --------------------------------------------------------------------------- +# update: migration state machine (client-side validation + PATCH shape) +# --------------------------------------------------------------------------- + + +UPDATE_MODULE = 'azure.cli.command_modules.acr.connected_registry' + + +class TestConnectedRegistryUpdateMigration(unittest.TestCase): + + def _patch_common(self): + p_validate = mock.patch(UPDATE_MODULE + '.validate_managed_registry', + return_value=(None, TEST_RG)) + p_subid = mock.patch(UPDATE_MODULE + '.get_subscription_id', + return_value=TEST_SUB) + return p_validate, p_subid + + def _run_update(self, current, **overrides): + client = mock.MagicMock() + kwargs = dict( + cmd=_make_cmd(), + client=client, + registry_name=TEST_REGISTRY, + connected_registry_name=TEST_CR, + resource_group_name=TEST_RG, + ) + kwargs.update(overrides) + p_validate, p_subid = self._patch_common() + with p_validate, p_subid, \ + mock.patch(UPDATE_MODULE + '.acr_connected_registry_show', return_value=current): + acr_connected_registry_update(**kwargs) + return client + + # ---- error paths ------------------------------------------------------ + + def test_identity_without_auth_type_errors(self): + cur = _fake_cr(auth_type=AUTH_TYPE_SYNC_TOKEN, + connection_state=CONNECTION_STATE_OFFLINE) + with self.assertRaises(ArgumentUsageError): + self._run_update(cur, identity=TEST_MSI_ID) + + def test_same_mode_rotation_rejected(self): + cur = _fake_cr(has_identity=True, connection_state=CONNECTION_STATE_OFFLINE) + with self.assertRaises(ArgumentUsageError) as ctx: + self._run_update(cur, auth_type=AUTH_TYPE_MANAGED_IDENTITY, + identity=TEST_MSI_ID2) + self.assertIn('already using', str(ctx.exception)) + + def test_not_offline_rejected(self): + cur = _fake_cr(auth_type=AUTH_TYPE_SYNC_TOKEN, connection_state='Online') + with self.assertRaises(ArgumentUsageError) as ctx: + self._run_update(cur, auth_type=AUTH_TYPE_MANAGED_IDENTITY, + identity=TEST_MSI_ID) + self.assertIn('Offline', str(ctx.exception)) + + def test_migrate_to_mi_requires_identity(self): + cur = _fake_cr(auth_type=AUTH_TYPE_SYNC_TOKEN, + connection_state=CONNECTION_STATE_OFFLINE) + with self.assertRaises(ArgumentUsageError): + self._run_update(cur, auth_type=AUTH_TYPE_MANAGED_IDENTITY) + + def test_migrate_to_mi_rejects_sync_token(self): + cur = _fake_cr(auth_type=AUTH_TYPE_SYNC_TOKEN, + connection_state=CONNECTION_STATE_OFFLINE) + with self.assertRaises(ArgumentUsageError): + self._run_update(cur, auth_type=AUTH_TYPE_MANAGED_IDENTITY, + identity=TEST_MSI_ID, sync_token_name='tok') + + def test_migrate_to_sync_token_requires_sync_token_name(self): + cur = _fake_cr(has_identity=True, connection_state=CONNECTION_STATE_OFFLINE) + with self.assertRaises(ArgumentUsageError): + self._run_update(cur, auth_type=AUTH_TYPE_SYNC_TOKEN) + + def test_migrate_to_sync_token_rejects_identity(self): + cur = _fake_cr(has_identity=True, connection_state=CONNECTION_STATE_OFFLINE) + with self.assertRaises(ArgumentUsageError): + self._run_update(cur, auth_type=AUTH_TYPE_SYNC_TOKEN, + identity=TEST_MSI_ID, sync_token_name='tok') + + def test_sync_token_without_auth_type_errors(self): + cur = _fake_cr(auth_type=AUTH_TYPE_SYNC_TOKEN, + connection_state=CONNECTION_STATE_OFFLINE) + with self.assertRaises(ArgumentUsageError): + self._run_update(cur, sync_token_name='tok') + + # ---- success paths: assert PATCH body shape --------------------------- + + def _extract_update_body(self, client): + # begin_update(resource_group_name=..., registry_name=..., + # connected_registry_name=..., connected_registry_update_parameters=...) + self.assertTrue(client.begin_update.called) + _, kwargs = client.begin_update.call_args + return kwargs['connected_registry_update_parameters'] + + def test_migrate_mi_to_sync_token_omits_identity(self): + """RP rejects any non-null identity in the PATCH body when auth_type=SyncToken. + This is the Bug 2 regression guard: `identity` in the PATCH must be None.""" + cur = _fake_cr(has_identity=True, connection_state=CONNECTION_STATE_OFFLINE) + client = self._run_update(cur, auth_type=AUTH_TYPE_SYNC_TOKEN, + sync_token_name='tok') + body = self._extract_update_body(client) + self.assertIsNone(body.identity) + self.assertEqual(body.sync_properties.auth_type, AUTH_TYPE_SYNC_TOKEN) + self.assertTrue(body.sync_properties.token_id.endswith('/tokens/tok')) + + def test_migrate_sync_token_to_mi_sends_identity(self): + cur = _fake_cr(auth_type=AUTH_TYPE_SYNC_TOKEN, + connection_state=CONNECTION_STATE_OFFLINE) + client = self._run_update(cur, auth_type=AUTH_TYPE_MANAGED_IDENTITY, + identity=TEST_MSI_ID) + body = self._extract_update_body(client) + self.assertIsNotNone(body.identity) + self.assertEqual(body.identity.type, MSI_TYPE_USER_ASSIGNED) + self.assertIn(TEST_MSI_ID, body.identity.user_assigned_identities) + self.assertEqual(body.sync_properties.auth_type, AUTH_TYPE_MANAGED_IDENTITY) + self.assertIsNone(body.sync_properties.token_id) + + +# --------------------------------------------------------------------------- +# delete: MI-mode skips sync-token / scope-map cleanup +# --------------------------------------------------------------------------- + + +class TestConnectedRegistryDeleteMI(unittest.TestCase): + + def _invoke(self, cleanup): + client = mock.MagicMock() + client.begin_delete.return_value.result.return_value = None + cr = _fake_cr(has_identity=True) + cr.parent.id = None + + with mock.patch(UPDATE_MODULE + '.validate_managed_registry', + return_value=(None, TEST_RG)), \ + mock.patch(UPDATE_MODULE + '.acr_connected_registry_show', + return_value=cr), \ + mock.patch(UPDATE_MODULE + '.get_token_from_id') as p_get_tok, \ + mock.patch(UPDATE_MODULE + '.cf_acr_tokens') as p_tok_client, \ + mock.patch(UPDATE_MODULE + '.cf_acr_scope_maps') as p_sm_client, \ + mock.patch(UPDATE_MODULE + '._update_ancestor_permissions') as p_anc: + acr_connected_registry_delete( + cmd=_make_cmd(), client=client, + connected_registry_name=TEST_CR, registry_name=TEST_REGISTRY, + cleanup=cleanup, yes=True, resource_group_name=TEST_RG) + + return client, p_get_tok, p_tok_client, p_sm_client, p_anc + + def test_mi_mode_skips_token_and_scope_map_cleanup(self): + # MI-mode must not resolve the sync token, instantiate token/scope-map + # clients, walk ancestors, or list siblings for cleanup. + client, p_get_tok, p_tok_client, p_sm_client, p_anc = self._invoke(cleanup=True) + p_get_tok.assert_not_called() + p_tok_client.assert_not_called() + p_sm_client.assert_not_called() + p_anc.assert_not_called() + client.list.assert_not_called() + + def test_mi_mode_delete_without_cleanup_is_noop_beyond_begin_delete(self): + client, p_get_tok, p_tok_client, p_sm_client, p_anc = self._invoke(cleanup=False) + p_get_tok.assert_not_called() + p_tok_client.assert_not_called() + p_sm_client.assert_not_called() + p_anc.assert_not_called() + client.list.assert_not_called() + + +# --------------------------------------------------------------------------- +# get-settings: MI-flavored connection string +# --------------------------------------------------------------------------- + + +class TestConnectedRegistryGetSettingsMI(unittest.TestCase): + + def _invoke(self, cr, **kw): + with mock.patch(UPDATE_MODULE + '.validate_managed_registry', + return_value=(None, TEST_RG)), \ + mock.patch(UPDATE_MODULE + '.acr_connected_registry_show', + return_value=cr): + return acr_connected_registry_get_settings( + cmd=_make_cmd(), client=mock.MagicMock(), + connected_registry_name=TEST_CR, registry_name=TEST_REGISTRY, + parent_protocol='https', resource_group_name=TEST_RG, **kw) + + def test_generate_password_rejected_on_mi(self): + cr = _fake_cr(has_identity=True) + with self.assertRaises(ArgumentUsageError): + self._invoke(cr, generate_password='1', yes=True) + + def test_missing_user_assigned_errors(self): + cr = _fake_cr(has_identity=True) + cr.identity.user_assigned_identities = None + with self.assertRaises(CLIError): + self._invoke(cr) + + def test_missing_client_id_errors(self): + cr = _fake_cr(has_identity=True, client_id=None) + with self.assertRaises(CLIError): + self._invoke(cr) + + def test_happy_path_returns_mi_keys(self): + cr = _fake_cr(has_identity=True, client_id='cid-happy') + result = self._invoke(cr) + self.assertEqual(result['ACR_MANAGED_IDENTITY_CLIENT_ID'], 'cid-happy') + self.assertEqual(result['ACR_MANAGED_IDENTITY_RESOURCE_ID'], TEST_MSI_ID) + self.assertIn('ManagedIdentityClientId=cid-happy', + result['ACR_REGISTRY_CONNECTION_STRING']) + self.assertNotIn('SYNC_TOKEN_USER', result) + + +# --------------------------------------------------------------------------- +# permissions show / update: blocked on MI +# --------------------------------------------------------------------------- + + +class TestConnectedRegistryPermissionsMI(unittest.TestCase): + + def test_permissions_show_blocked_on_mi(self): + cr = _fake_cr(has_identity=True) + with mock.patch(UPDATE_MODULE + '.validate_managed_registry', + return_value=(None, TEST_RG)), \ + mock.patch(UPDATE_MODULE + '.acr_connected_registry_show', + return_value=cr): + with self.assertRaises(ArgumentUsageError): + acr_connected_registry_permissions_show( + cmd=_make_cmd(), client=mock.MagicMock(), + connected_registry_name=TEST_CR, registry_name=TEST_REGISTRY, + resource_group_name=TEST_RG) + + def test_permissions_update_blocked_on_mi(self): + cr = _fake_cr(has_identity=True) + client = mock.MagicMock() + client.list.return_value = [cr] + with mock.patch(UPDATE_MODULE + '.validate_managed_registry', + return_value=(None, TEST_RG)): + with self.assertRaises(ArgumentUsageError): + acr_connected_registry_permissions_update( + cmd=_make_cmd(), client=client, + connected_registry_name=TEST_CR, registry_name=TEST_REGISTRY, + add_repos=['r1'], resource_group_name=TEST_RG) + + +if __name__ == '__main__': + unittest.main() diff --git a/src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_connectedregistry_commands.py b/src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_connectedregistry_commands.py index 67bdaaf8f30..12cbb62cd6d 100644 --- a/src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_connectedregistry_commands.py +++ b/src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_connectedregistry_commands.py @@ -234,3 +234,76 @@ def test_acr_connectedregistry_dedicated_endpoint_not_enabled(self, resource_gro # Delete registry self.cmd('acr delete -n {registry_name} -g {rg} -y') + + @AllowLargeResponse(size_kb=99999) + @record_only() + def test_acr_connectedregistry_managed_identity(self): + # Managed-identity auth mode for connected registries. Recorded scenario is + # intentionally minimal (create + show + get-settings + delete) so the cassette + # does not depend on the SyncToken<->ManagedIdentity migration round-trip. + # Client-side validation, the migration state machine, and MI-mode delete cleanup + # are covered by mock-based unit tests in test_acr_connected_registry_mi_unit.py. + # Note: This test does NOT import from azure.mgmt.containerregistry so the + # recording remains valid across future SDK model-namespace changes. + # + # Recording status: + # * @record_only() keeps this test in playback-only mode in CI. + # * Cassette pending: MI is currently deployed only to dogfood, and the dogfood + # control-plane / ARM has been broadly 500-flaky (see repro notes in the + # ship-todo doc). Re-record against prod once the preview API is deployed + # there. To re-record: remove @record_only, run with + # AZURE_TEST_RUN_LIVE=True against the target cloud, sanitize, restore + # @record_only, commit both the test and the cassette. + # Preparer bypass: to avoid the sub-wide RG-quota / ARM LIST flake, this test is + # pinned to a pre-existing shared RG (adjust `rg` below when re-recording). + self.kwargs.update({ + 'rg': 'shaoren-rg', + 'registry_name': self.create_random_name('clireg', 20), + 'cr_mi_name': 'crmiscen', + 'rg_loc': 'australiaeast', + 'sku': 'Premium', + 'identity_name': self.create_random_name('cr-mi-id', 20), + }) + + # Registry + data endpoint (required for connected-registry). + # Pass -g on every acr command: without it, the CLI falls back to + # `Microsoft.Resources/resources` sub-wide LIST which is currently 500-flaky in + # dogfood ARM. -g routes straight to the RP. + self.cmd('acr create -n {registry_name} -g {rg} -l {rg_loc} --sku {sku}', + checks=[self.check('name', '{registry_name}'), + self.check('sku.name', '{sku}'), + self.check('provisioningState', 'Succeeded')]) + self.cmd('acr update -n {registry_name} -g {rg} --data-endpoint-enabled true', + checks=self.check('dataEndpointEnabled', True)) + + # User-assigned identity. + result = self.cmd('identity create --name {identity_name} -g {rg}').get_output_in_json() + self.kwargs['identity_id'] = result['id'] + + # --- Create with ManagedIdentity --- + self.cmd('acr connected-registry create -n {cr_mi_name} -r {registry_name} -g {rg} ' + '-m ReadOnly --auth-type ManagedIdentity --identity {identity_id}', + checks=[self.check('name', '{cr_mi_name}'), + self.check('mode', 'ReadOnly'), + self.check('provisioningState', 'Succeeded'), + self.check('parent.syncProperties.authType', 'ManagedIdentity'), + self.check('identity.type', 'UserAssigned')]) + + # --- Show reflects MI --- + self.cmd('acr connected-registry show -n {cr_mi_name} -r {registry_name} -g {rg}', + checks=[self.check('parent.syncProperties.authType', 'ManagedIdentity'), + self.check('identity.type', 'UserAssigned')]) + + # --- get-settings returns MI-flavored connection string --- + settings = self.cmd('acr connected-registry get-settings -n {cr_mi_name} -r {registry_name} ' + '-g {rg} --parent-protocol https').get_output_in_json() + self.assertIn('ACR_MANAGED_IDENTITY_CLIENT_ID', settings) + self.assertIn('ACR_MANAGED_IDENTITY_RESOURCE_ID', settings) + self.assertIn('ManagedIdentityClientId=', settings['ACR_REGISTRY_CONNECTION_STRING']) + self.assertNotIn('SYNC_TOKEN_USER', settings) + + # --- MI-mode delete (no sync token / scope map cleanup path) --- + self.cmd('acr connected-registry delete -n {cr_mi_name} -r {registry_name} -g {rg} -y') + self.cmd('acr delete -n {registry_name} -g {rg} -y') + # Shared RG: clean up the user-assigned identity too. + self.cmd('identity delete --name {identity_name} -g {rg}') diff --git a/src/azure-cli/setup.py b/src/azure-cli/setup.py index 5549c930e8a..4918bedbbfd 100644 --- a/src/azure-cli/setup.py +++ b/src/azure-cli/setup.py @@ -78,7 +78,7 @@ 'azure-mgmt-cognitiveservices==15.0.0b4', # 15.0.0b4 version supports compute operations which is in PrPr, pls don't remove it without consulting our team 'azure-mgmt-compute~=34.1.0', 'azure-mgmt-containerinstance==10.2.0b1', - 'azure-mgmt-containerregistry==15.1.0b2', + 'azure-mgmt-containerregistry==15.1.0b3', 'azure-mgmt-containerregistrytasks==1.0.0b1', 'azure-mgmt-containerservice~=41.5.0', 'azure-mgmt-cosmosdb==9.9.0',