[ARO] downstream extension

src/azure-cli/azure/cli/command_modules/aro/__init__.py

@@ -3,15 +3,20 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
 
-from azure.cli.core import AzCommandsLoader, ModExtensionSuppress
-
-import azure.cli.command_modules.aro._help  # pylint: disable=unused-import
+from azure.cli.command_modules.aro._client_factory import cf_aro
+from azure.cli.command_modules.aro._params import load_arguments
+from azure.cli.command_modules.aro.commands import load_command_table
+from azure.cli.core import (
+    AzCommandsLoader,
+    ModExtensionSuppress
+)
+from azure.cli.core.commands import CliCommandType
+from azure.cli.core.aaz import load_aaz_command_table
+from . import aaz
 
 
 class AroCommandsLoader(AzCommandsLoader):
     def __init__(self, cli_ctx=None):
-        from azure.cli.core.commands import CliCommandType
-        from azure.cli.command_modules.aro._client_factory import cf_aro
         aro_custom = CliCommandType(
             operations_tmpl='azure.cli.command_modules.aro.custom#{}',
             client_factory=cf_aro)
@@ -23,23 +28,15 @@ def __init__(self, cli_ctx=None):
                          custom_command_type=aro_custom)
 
     def load_command_table(self, args):
-        from azure.cli.command_modules.aro.commands import load_command_table
-        from azure.cli.core.aaz import load_aaz_command_table
-        try:
-            from . import aaz
-        except ImportError:
-            aaz = None
-        if aaz:
-            load_aaz_command_table(
-                loader=self,
-                aaz_pkg_name=aaz.__name__,
-                args=args
-            )
+        load_aaz_command_table(
+            loader=self,
+            aaz_pkg_name=aaz.__name__,
+            args=args
+        )
         load_command_table(self, args)
         return self.command_table
 
     def load_arguments(self, command):
-        from azure.cli.command_modules.aro._params import load_arguments
         load_arguments(self, command)
 
 

src/azure-cli/azure/cli/command_modules/aro/_actions.py

@@ -5,6 +5,7 @@
 
 import argparse
 
+from azure.mgmt.redhatopenshift.models import PlatformWorkloadIdentity
 from azure.cli.core.azclierror import CLIError
 
 
@@ -13,7 +14,6 @@
 class AROPlatformWorkloadIdentityAddAction(argparse._AppendAction):
 
     def __call__(self, parser, namespace, values, option_string=None):
-        from azure.mgmt.redhatopenshift.models import PlatformWorkloadIdentity
         try:
             if len(values) != 2:
                 msg = f"{option_string} requires 2 values in format: `OPERATOR_NAME RESOURCE_ID`"

src/azure-cli/azure/cli/command_modules/aro/_client_factory.py

@@ -3,8 +3,14 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
 
+from azure.mgmt.redhatopenshift import AzureRedHatOpenShiftClient
+from azure.cli.core.commands.client_factory import get_mgmt_service_client
+
 
 def cf_aro(cli_ctx, *_):
-    from azure.cli.core.commands.client_factory import get_mgmt_service_client
-    from azure.mgmt.redhatopenshift import AzureRedHatOpenShiftClient
-    return get_mgmt_service_client(cli_ctx, AzureRedHatOpenShiftClient)
+    opt_args = {}
+
+    client = get_mgmt_service_client(
+        cli_ctx, AzureRedHatOpenShiftClient, base_url_bound=False, **opt_args)
+
+    return client

src/azure-cli/azure/cli/command_modules/aro/_dynamic_validators.py

@@ -8,22 +8,18 @@
 import re
 from itertools import tee
 
+from azure.cli.command_modules.aro._rbac import has_role_assignment_on_resource
+from azure.cli.command_modules.aro._validators import validate_vnet, validate_cidr
+from azure.cli.command_modules.aro.aaz.latest.network.vnet import Show as vnet_show
+from azure.cli.command_modules.aro.aaz.latest.network.vnet.subnet import Show as subnet_show
+from azure.cli.core.azclierror import CLIInternalError, InvalidArgumentValueError, RequiredArgumentMissingError
 from azure.cli.core.commands.client_factory import get_mgmt_service_client
 from azure.cli.core.commands.validators import get_default_location_from_resource_group
 from azure.cli.core.profiles import ResourceType
-from azure.cli.core.azclierror import (
-    CLIInternalError,
-    InvalidArgumentValueError,
-    RequiredArgumentMissingError
-)
 from azure.core.exceptions import HttpResponseError, ResourceNotFoundError
 from azure.mgmt.core.tools import is_valid_resource_id, parse_resource_id
-from azure.cli.command_modules.aro._validators import validate_vnet, validate_cidr
-from azure.cli.command_modules.aro._rbac import has_role_assignment_on_resource
-from azure.cli.command_modules.aro.aaz.latest.network.vnet.subnet import Show as subnet_show
-from azure.cli.command_modules.aro.aaz.latest.network.vnet import Show as vnet_show
-
 from knack.log import get_logger
+import azure.cli.command_modules.aro.custom
 
 
 logger = get_logger(__name__)
@@ -334,8 +330,7 @@ def _validate_version(cmd,
         if namespace.location is None:
             get_default_location_from_resource_group(cmd, namespace)
 
-        from azure.cli.command_modules.aro.custom import aro_get_versions
-        versions = aro_get_versions(namespace.client, namespace.location)
+        versions = azure.cli.command_modules.aro.custom.aro_get_versions(namespace.client, namespace.location)
 
         found = False
         for version in versions:

src/azure-cli/azure/cli/command_modules/aro/_help.py

@@ -100,3 +100,20 @@
   short-summary: Wait for a cluster to reach a desired state.
   long-summary: If an operation on a cluster was interrupted or was started with `--no-wait`, use this command to wait for it to complete.
 """
+
+helps['aro identity'] = """
+  type: group
+  short-summary: Manage identities related to Azure Red Hat OpenShift clusters.
+"""
+
+helps['aro identity get-required'] = """
+  type: command
+  short-summary: Get required identities
+  long-summary: Get required identities for creating a cluster with managed identities.
+"""
+
+helps['aro identity create-required'] = """
+  type: command
+  short-summary: Create required identities
+  long-summary: Create required identities to prepare for creating a cluster with managed identities.
+"""

src/azure-cli/azure/cli/command_modules/aro/_params.py

@@ -100,7 +100,7 @@ def load_arguments(self, _):
                    options_list=['--worker-encryption-at-host', '--worker-enc-host'],
                    help='Encryption at host flag for worker VMs. [Default: false]')
         c.argument('worker_vm_size',
-                   help='Size of worker VMs.')
+                   help='Size of worker VMs. [Default: Standard_D4s_v5]')
         c.argument('worker_vm_disk_size_gb',
                    type=int,
                    help='Disk size in GB of worker VMs. [Default: 128]',

src/azure-cli/azure/cli/command_modules/aro/_rbac.py

@@ -3,66 +3,60 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
 
+import typing
 import uuid
 
+from azure.cli.command_modules.aro.aaz.latest.identity import Create as _create_identity
+from azure.cli.command_modules.aro.aaz.latest.role.assignment import Create as _role_assignment_create
 from azure.cli.core.commands.client_factory import (
     get_mgmt_service_client,
     get_subscription_id
 )
-from azure.cli.core.profiles import (
-    get_sdk,
-    ResourceType
-)
+from azure.cli.core.profiles import ResourceType
+from azure.core.exceptions import ResourceExistsError
 from azure.mgmt.core.tools import resource_id
 from knack.log import get_logger
-from msrest.exceptions import ValidationError
 
 ROLE_NETWORK_CONTRIBUTOR = '4d97b98b-1d4f-4787-a291-c67834d212e7'
 ROLE_READER = 'acdd72a7-3385-48ef-bd42-f606fba81ae7'
 
 logger = get_logger(__name__)
 
 
-def _gen_uuid():
+# UUID/GUID generation is extracted into a function to aid in testing.
+def _gen_uuid() -> uuid.UUID:
     return uuid.uuid4()
 
 
-def _create_role_assignment(auth_client, resource, params):
-    # retry "ValidationError: A hash conflict was encountered for the role Assignment ID. Please use a new Guid."
-    max_retries = 3
-    retries = 0
-    while True:
-        try:
-            return auth_client.role_assignments.create(resource, _gen_uuid(), params)
-        except ValidationError as ex:
-            if retries >= max_retries:
-                raise
-            retries += 1
-            logger.warning("%s; retry %d of %d", ex, retries, max_retries)
-
-
-def assign_role_to_resource(cli_ctx, resource, object_id, role_name):
-    auth_client = get_mgmt_service_client(cli_ctx, ResourceType.MGMT_AUTHORIZATION)
+def create_identity(cmd, location, group, name) -> typing.Any:
+    create = _create_identity(cli_ctx=cmd.cli_ctx)
 
-    RoleAssignmentCreateParameters = get_sdk(cli_ctx, ResourceType.MGMT_AUTHORIZATION,
-                                             'RoleAssignmentCreateParameters', mod='models',
-                                             operation_group='role_assignments')
+    # idempotent
+    return create(command_args={
+        "location": location,
+        "resource_group": group,
+        "resource_name": name,
+    })
 
-    role_definition_id = resource_id(
-        subscription=get_subscription_id(cli_ctx),
-        namespace='Microsoft.Authorization',
-        type='roleDefinitions',
-        name=role_name,
-    )
 
-    _create_role_assignment(auth_client, resource, RoleAssignmentCreateParameters(
-        role_definition_id=role_definition_id,
-        principal_id=object_id,
-        principal_type='ServicePrincipal',
-    ))
+def create_role_assignment(cli_ctx, principal_id, role_definition_id, scope) -> typing.Any | None:
+    create = _role_assignment_create(cli_ctx=cli_ctx)
+    try:
+        return create(command_args={
+            "principal_id": principal_id,
+            "principal_type": "ServicePrincipal",
+            "role_definition_id": role_definition_id,
+            "scope": scope,
+            "role_assignment_name": str(_gen_uuid()),
+        })
+    except ResourceExistsError:
+        logger.warning("Role Assignment already exists for "
+                       "{ principal: %s, role definition: %s, scope: %s }.",
+                       principal_id, role_definition_id, scope)
+        return None
 
 
-def has_role_assignment_on_resource(cli_ctx, resource, object_id, role_name):
+def has_role_assignment_on_resource(cli_ctx, resource, object_id, role_name) -> bool:
     auth_client = get_mgmt_service_client(cli_ctx, ResourceType.MGMT_AUTHORIZATION)
 
     role_definition_id = resource_id(
@@ -78,3 +72,19 @@ def has_role_assignment_on_resource(cli_ctx, resource, object_id, role_name):
             return True
 
     return False
+
+
+def print_identity_create_cmd(group, name, location) -> None:
+    msg = f"    az identity create -g '{group}' -n '{name}' -l '{location}'"
+    logger.warning(msg)
+
+
+def print_role_assignment_create_cmd(assignee, role, scope) -> None:
+    msg = [
+        "    az role assignment create",
+        f'--assignee-object-id "{assignee}"',
+        "--assignee-principal-type ServicePrincipal",
+        f"--role '{role}'",
+        f'--scope "{scope}"',
+    ]
+    logger.warning(" ".join(msg))

src/azure-cli/azure/cli/command_modules/aro/_validators.py

@@ -10,18 +10,17 @@
 from os.path import exists
 from collections import Counter
 
-from azure.cli.core.commands.client_factory import get_mgmt_service_client, get_subscription_id
-from azure.cli.core.profiles import ResourceType
+from azure.cli.command_modules.aro.aaz.latest.network.vnet.subnet import Show as subnet_show
 from azure.cli.core.azclierror import (
     CLIInternalError,
     InvalidArgumentValueError,
     RequiredArgumentMissingError,
     MutuallyExclusiveArgumentError
 )
+from azure.cli.core.commands.client_factory import get_mgmt_service_client, get_subscription_id
+from azure.cli.core.profiles import ResourceType
 from azure.core.exceptions import ResourceNotFoundError, HttpResponseError
 from azure.mgmt.core.tools import is_valid_resource_id, parse_resource_id, resource_id
-from azure.cli.command_modules.aro.aaz.latest.network.vnet.subnet import Show as subnet_show
-
 from knack.log import get_logger
 
 logger = get_logger(__name__)
@@ -45,7 +44,8 @@ def _validate_client_id(namespace):
         if namespace.client_id is None:
             return
         if hasattr(namespace, 'enable_managed_identity') and namespace.enable_managed_identity is True:
-            raise MutuallyExclusiveArgumentError('Must not specify --client-id when --enable-managed-identity is True')  # pylint: disable=line-too-long
+            raise MutuallyExclusiveArgumentError("Must not specify --client-id when "
+                                                 "--enable-managed-identity/--enable-mi is True")
         if namespace.platform_workload_identities is not None:
             raise MutuallyExclusiveArgumentError('Must not specify --client-id when --assign-platform-workload-identity is used')  # pylint: disable=line-too-long
         try:
@@ -65,7 +65,8 @@ def _validate_client_secret(namespace):
         if namespace.client_secret is None:
             return
         if hasattr(namespace, 'enable_managed_identity') and namespace.enable_managed_identity is True:
-            raise MutuallyExclusiveArgumentError('Must not specify --client-secret when --enable-managed-identity is True')  # pylint: disable=line-too-long
+            raise MutuallyExclusiveArgumentError("Must not specify --client-secret when "
+                                                 "--enable-managed-identity/--enable-mi is True")
         if namespace.platform_workload_identities is not None:
             raise MutuallyExclusiveArgumentError('Must not specify --client-secret when --assign-platform-workload-identity is used')  # pylint: disable=line-too-long
         if isCreate and (namespace.client_id is None or not str(namespace.client_id)):
@@ -321,17 +322,27 @@ def validate_enable_managed_identity(namespace):
     if not namespace.enable_managed_identity:
         return
 
-    if namespace.client_id is not None:
-        raise InvalidArgumentValueError('Must not specify --client-id when --enable-managed-identity is True')
+    if namespace.client_id:
+        raise InvalidArgumentValueError("Must not specify --client-id when "
+                                        "--enable-managed-identity/--enable-mi is True")
+
+    if namespace.client_secret:
+        raise InvalidArgumentValueError("Must not specify --client-secret when "
+                                        "--enable-managed-identity/--enable-mi is True")
 
-    if namespace.client_secret is not None:
-        raise InvalidArgumentValueError('Must not specify --client-secret when --enable-managed-identity is True')
+    if namespace.platform_workload_identities and not namespace.mi_user_assigned:
+        raise RequiredArgumentMissingError("Must specify --assign-cluster-identity/--mi-user-assigned "
+                                           "with --assign-platform-workload-identity/--assign-platform-wi")
 
-    if not namespace.platform_workload_identities:
-        raise RequiredArgumentMissingError('Enabling managed identity requires platform workload identities to be provided')  # pylint: disable=line-too-long
+    if not namespace.platform_workload_identities and namespace.mi_user_assigned:
+        raise RequiredArgumentMissingError("Must specify --assign-platform-workload-identity/--assign-platform-wi "
+                                           "with --assign-cluster-identity/--mi-user-assigned")
 
-    if not namespace.mi_user_assigned:
-        raise RequiredArgumentMissingError('Enabling managed identity requires cluster identity to be provided')
+    if not namespace.platform_workload_identities and not namespace.mi_user_assigned and not namespace.version:
+        raise RequiredArgumentMissingError("--enable-managed-identity/--enable-mi without "
+                                           "--assign-cluster-identity/--mi-user-assigned and "
+                                           "--assign-platform-wi/--assign-platform-workload-identity "
+                                           "requires --version.")
 
 
 def validate_platform_workload_identities(isCreate):

src/azure-cli/azure/cli/command_modules/aro/aaz/latest/__init__.py

@@ -4,3 +4,7 @@
 #
 # Code generated by aaz-dev-tools
 # --------------------------------------------------------------------------------------------
+
+# pylint: skip-file
+# flake8: noqa
+

src/azure-cli/azure/cli/command_modules/aro/aaz/latest/identity/__cmd_group.py

@@ -10,6 +10,7 @@
 
 from azure.cli.core.aaz import *
 
+
 class __CMDGroup(AAZCommandGroup):
     """Manage Managed Identity
     """

src/azure-cli/azure/cli/command_modules/aro/aaz/latest/identity/_create.py

@@ -10,6 +10,7 @@
 
 from azure.cli.core.aaz import *
 
+
 class Create(AAZCommand):
     """Create an identity in the specified subscription and resource group.
     """

src/azure-cli/azure/cli/command_modules/aro/aaz/latest/identity/_delete.py

@@ -10,6 +10,7 @@
 
 from azure.cli.core.aaz import *
 
+
 class Delete(AAZCommand):
     """Delete the identity.
     """

src/azure-cli/azure/cli/command_modules/aro/aaz/latest/network/vnet/__init__.py

@@ -9,4 +9,4 @@
 # flake8: noqa
 
 from .__cmd_group import *
-from ._show import *
+from ._show import *