Add new recordings after rebasing

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

@@ -3,110 +3,55 @@
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
 
-import datetime
 import time
-import uuid
 
-from azure.cli.core._profile import Profile
-from azure.cli.core.commands.client_factory import configure_common_settings
-from azure.cli.core.azclierror import BadRequestError
-from azure.graphrbac import GraphRbacManagementClient
-from azure.graphrbac.models import ApplicationCreateParameters
-from azure.graphrbac.models import GraphErrorException
-from azure.graphrbac.models import PasswordCredential
-from azure.graphrbac.models import ServicePrincipalCreateParameters
+from azure.cli.command_modules.role import graph_client_factory
+from azure.cli.command_modules.role import GraphError
+
 from knack.log import get_logger
 
 logger = get_logger(__name__)
 
 
+# All methods now return the text based values
+# for consistency and hiding implementation details
 class AADManager:
     def __init__(self, cli_ctx):
-        profile = Profile(cli_ctx=cli_ctx)
-        self.cli_ctx = cli_ctx
-        credentials, _, tenant_id = profile.get_login_credentials(
-            resource=cli_ctx.cloud.endpoints.active_directory_graph_resource_id)
-        self.client = GraphRbacManagementClient(
-            credentials, tenant_id, base_url=cli_ctx.cloud.endpoints.active_directory_graph_resource_id)
-        configure_common_settings(cli_ctx, self.client)
+        self.client = graph_client_factory(cli_ctx)
 
     def create_application(self, display_name):
-        password = uuid.uuid4()
-        start_date = datetime.datetime.utcnow()
-        end_date = datetime.datetime(2299, 12, 31, tzinfo=datetime.timezone.utc)
-
-        app = self.client.applications.create(ApplicationCreateParameters(
-            display_name=display_name,
-            identifier_uris=[],
-            password_credentials=[
-                PasswordCredential(
-                    custom_key_identifier=str(start_date).encode(),
-                    start_date=start_date,
-                    end_date=end_date,
-                    value=password,
-                ),
-            ],
-        ))
-
-        return app, password
-
-    def get_service_principal(self, app_id):
-        sps = list(self.client.service_principals.list(
-            filter=f"appId eq '{app_id}'"))
+        request_body = {"displayName": display_name}
+        app = self.client.application_create(request_body)
+        obj_id = app["id"]
+        app_id = app["appId"]
+        password = self.add_password(obj_id)
+        return app_id, password
+
+    def get_service_principal_id(self, app_id):
+        sps = self.client.service_principal_list(f"appId eq '{app_id}'")
         if sps:
-            return sps[0]
+            return sps[0]["id"]
         return None
 
-    def get_application_by_client_id(self, client_id):
-        apps = list(self.client.applications.list(
-            filter=f"appId eq '{client_id}'"))
+    def get_application_object_id_by_client_id(self, client_id):
+        apps = self.client.application_list(f"appId eq '{client_id}'")
         if apps:
-            return apps[0]
+            return apps[0]["id"]
         return None
 
     def create_service_principal(self, app_id):
         max_retries = 3
         retries = 0
         while True:
             try:
-                return self.client.service_principals.create(
-                    ServicePrincipalCreateParameters(app_id=app_id))
-            except GraphErrorException as ex:
+                return self.client.service_principal_create({"appId": app_id})["id"]
+            except GraphError as ex:
                 if retries >= max_retries:
                     raise
                 retries += 1
                 logger.warning("%s; retry %d of %d", ex, retries, max_retries)
                 time.sleep(10)
 
-    def refresh_application_credentials(self, object_id):
-        password = uuid.uuid4()
-        key_id = uuid.uuid4()
-        start_date = datetime.datetime.utcnow()
-        end_date = datetime.datetime(2299, 12, 31, tzinfo=datetime.timezone.utc)
-
-        try:
-            credentials = list(self.client.applications.list_password_credentials(object_id))
-        except GraphErrorException as e:
-            logger.error(e.message)
-            raise
-
-        # keys created with older version of cli are not updatable
-        # https://github.com/Azure/azure-sdk-for-python/issues/18131
-        for c in credentials:
-            if c.custom_key_identifier is None:
-                raise BadRequestError("Cluster AAD application contains a client secret with an empty identifier.\n\
-Please either manually remove the existing client secret and run `az aro update --refresh-credentials`, \n\
-or manually create a new client secret and run `az aro update --client-secret <ClientSecret>`.")
-
-        # when appending credentials ALL fields must be present, otherwise
-        # azure gives ambiguous errors about not being able to update old keys
-        credentials.append(PasswordCredential(
-            custom_key_identifier=str(start_date).encode(),  # bytearray
-            key_id=str(key_id),
-            start_date=start_date,
-            end_date=end_date,
-            value=password))
-
-        self.client.applications.update_password_credentials(object_id, credentials)
-
-        return password
+    def add_password(self, obj_id):
+        cred = self.client.application_add_password(obj_id, {})
+        return cred["secretText"]

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

@@ -18,7 +18,7 @@
     has_role_assignment_on_resource
 from azure.cli.command_modules.aro._rbac import ROLE_NETWORK_CONTRIBUTOR, ROLE_READER
 from azure.cli.command_modules.aro._validators import validate_subnets
-from azure.graphrbac.models import GraphErrorException
+from azure.cli.command_modules.role import GraphError
 
 from knack.log import get_logger
 
@@ -75,15 +75,14 @@ def aro_create(cmd,  # pylint: disable=too-many-locals
 
     aad = AADManager(cmd.cli_ctx)
     if client_id is None:
-        app, client_secret = aad.create_application(cluster_resource_group or 'aro-' + random_id)
-        client_id = app.app_id
+        client_id, client_secret = aad.create_application(cluster_resource_group or 'aro-' + random_id)
 
-    client_sp = aad.get_service_principal(client_id)
-    if not client_sp:
-        client_sp = aad.create_service_principal(client_id)
+    client_sp_id = aad.get_service_principal_id(client_id)
+    if not client_sp_id:
+        client_sp_id = aad.create_service_principal(client_id)
 
-    rp_client_sp = aad.get_service_principal(resolve_rp_client_id())
-    if not rp_client_sp:
+    rp_client_sp_id = aad.get_service_principal_id(resolve_rp_client_id())
+    if not rp_client_sp_id:
         raise ResourceNotFoundError("RP service principal not found.")
 
     worker_vm_size = worker_vm_size or 'Standard_D4s_v3'
@@ -140,7 +139,7 @@ def aro_create(cmd,  # pylint: disable=too-many-locals
         ],
     )
 
-    sp_obj_ids = [client_sp.object_id, rp_client_sp.object_id]
+    sp_obj_ids = [client_sp_id, rp_client_sp_id]
     ensure_resource_permissions(cmd.cli_ctx, oc, True, sp_obj_ids)
 
     return sdk_no_wait(no_wait, client.begin_create_or_update,
@@ -151,7 +150,7 @@ def aro_create(cmd,  # pylint: disable=too-many-locals
 
 def aro_delete(cmd, client, resource_group_name, resource_name, no_wait=False):
     # TODO: clean up rbac
-    rp_client_sp = None
+    rp_client_sp_id = None
 
     try:
         oc = client.get(resource_group_name, resource_name)
@@ -166,16 +165,16 @@ def aro_delete(cmd, client, resource_group_name, resource_name, no_wait=False):
 
     # Best effort - assume the role assignments on the SP exist if exception raised
     try:
-        rp_client_sp = aad.get_service_principal(resolve_rp_client_id())
-        if not rp_client_sp:
+        rp_client_sp_id = aad.get_service_principal_id(resolve_rp_client_id())
+        if not rp_client_sp_id:
             raise ResourceNotFoundError("RP service principal not found.")
-    except GraphErrorException as e:
+    except GraphError as e:
         logger.info(e.message)
 
     # Customers frequently remove the Cluster or RP's service principal permissions.
     # Attempt to fix this before performing any action against the cluster
-    if rp_client_sp:
-        ensure_resource_permissions(cmd.cli_ctx, oc, False, [rp_client_sp.object_id])
+    if rp_client_sp_id:
+        ensure_resource_permissions(cmd.cli_ctx, oc, False, [rp_client_sp_id])
 
     return sdk_no_wait(no_wait, client.begin_delete,
                        resource_group_name=resource_group_name,
@@ -315,8 +314,8 @@ def cluster_application_update(cli_ctx,
                                refresh_cluster_credentials):
     # QUESTION: is there possible unification with the create path?
 
-    rp_client_sp = None
-    client_sp = None
+    rp_client_sp_id = None
+    client_sp_id = None
     random_id = generate_random_id()
 
     # if any of these are set - we expect users to have access to fix rbac so we fail
@@ -327,10 +326,10 @@ def cluster_application_update(cli_ctx,
 
     # check if we can see if RP service principal exists
     try:
-        rp_client_sp = aad.get_service_principal(resolve_rp_client_id())
-        if not rp_client_sp:
+        rp_client_sp_id = aad.get_service_principal_id(resolve_rp_client_id())
+        if not rp_client_sp_id:
             raise ResourceNotFoundError("RP service principal not found.")
-    except GraphErrorException as e:
+    except GraphError as e:
         if fail:
             logger.error(e.message)
             raise
@@ -341,33 +340,32 @@ def cluster_application_update(cli_ctx,
     # If application does not exist - creates new one
     if refresh_cluster_credentials:
         try:
-            app = aad.get_application_by_client_id(client_id or oc.service_principal_profile.client_id)
+            app = aad.get_application_object_id_by_client_id(client_id or oc.service_principal_profile.client_id)
             if not app:
                 # we were not able to find and applications, create new one
                 parts = parse_resource_id(oc.cluster_profile.resource_group_id)
                 cluster_resource_group = parts['resource_group']
 
-                app, client_secret = aad.create_application(cluster_resource_group or 'aro-' + random_id)
-                client_id = app.app_id
+                client_id, client_secret = aad.create_application(cluster_resource_group or 'aro-' + random_id)
             else:
-                client_secret = aad.refresh_application_credentials(app.object_id)
-        except GraphErrorException as e:
+                client_secret = aad.add_password(app)
+        except GraphError as e:
             logger.error(e.message)
             raise
 
     # attempt to get/create SP if one was not found.
     try:
-        client_sp = aad.get_service_principal(client_id or oc.service_principal_profile.client_id)
-    except GraphErrorException as e:
+        client_sp_id = aad.get_service_principal_id(client_id or oc.service_principal_profile.client_id)
+    except GraphError as e:
         if fail:
             logger.error(e.message)
             raise
         logger.info(e.message)
 
-    if fail and not client_sp:
-        client_sp = aad.create_service_principal(client_id or oc.service_principal_profile.client_id)
+    if fail and not client_sp_id:
+        client_sp_id = aad.create_service_principal(client_id or oc.service_principal_profile.client_id)
 
-    sp_obj_ids = [sp.object_id for sp in [rp_client_sp, client_sp] if sp]
+    sp_obj_ids = [sp for sp in [rp_client_sp_id, client_sp_id] if sp]
     ensure_resource_permissions(cli_ctx, oc, fail, sp_obj_ids)
 
     return client_id, client_secret
@@ -403,4 +401,4 @@ def ensure_resource_permissions(cli_ctx, oc, fail, sp_obj_ids):
                         raise
                     logger.info(e.message)
                 if not resource_contributor_exists:
-                    assign_role_to_resource(cli_ctx, resource, sp_id, role)
+                    assign_role_to_resource(cli_ctx, resource, sp_id, role)