Fixed naming, test output, and code duplication.

hack/e2e/run-rp-and-e2e.sh

@@ -66,7 +66,7 @@ deploy_e2e_deps() {
     az network route-table create \
       -g "$ARO_RESOURCEGROUP" \
       -l "$LOCATION" \
-      -n "$CLUSTER-rt"
+      -n "$CLUSTER-rt" >/dev/null
 
     echo "########## Create ARO Subnet ##########"
     for subnet in "$CLUSTER-master" "$CLUSTER-worker"; do
@@ -83,8 +83,8 @@ deploy_e2e_deps() {
       -g "$ARO_RESOURCEGROUP" \
       --vnet-name dev-vnet \
       -n "$CLUSTER-master" \
-      --disable-private-link-service-network-policies true >/dev/null \
-      --route-table "$CLUSTER-rt"
+      --route-table "$CLUSTER-rt" \
+      --disable-private-link-service-network-policies true >/dev/null
 
     echo "########## Create Cluster SPN ##########"
     az ad sp create-for-rbac -n "$CLUSTER-$LOCATION" --role contributor \

python/az/aro/azext_aro/_rbac.py

@@ -33,7 +33,7 @@ def assign_contributor_to_vnet(cli_ctx, vnet, object_id):
         type='roleDefinitions',
         name=DEVELOPMENT_CONTRIBUTOR if rp_mode_development() else CONTRIBUTOR,
     )
-    if has_assignment(list(client.role_assignments.list_for_scope(vnet)), role_definition_id, object_id):
+    if has_assignment(client.role_assignments.list_for_scope(vnet), role_definition_id, object_id):
         return
 
     # generate random uuid for role assignment
@@ -46,7 +46,7 @@ def assign_contributor_to_vnet(cli_ctx, vnet, object_id):
     ))
 
 
-def assign_permissions_to_routetable(cli_ctx, master_subnet, worker_subnet, object_id):
+def assign_contributor_to_routetable(cli_ctx, master_subnet, worker_subnet, object_id):
     auth_client = get_mgmt_service_client(cli_ctx, ResourceType.MGMT_AUTHORIZATION)
     client = get_mgmt_service_client(cli_ctx, ResourceType.MGMT_NETWORK)
 
@@ -57,16 +57,16 @@ def assign_permissions_to_routetable(cli_ctx, master_subnet, worker_subnet, obje
         name=DEVELOPMENT_CONTRIBUTOR if rp_mode_development() else CONTRIBUTOR,
     )
 
-    route_tables = []
+    route_tables = set()
     for sn in [master_subnet, worker_subnet]:
         sid = parse_resource_id(sn)
         subnet = client.subnets.get(resource_group_name=sid['resource_group'],
                                     virtual_network_name=sid['name'],
                                     subnet_name=sid['resource_name'])
         if subnet.route_table is not None and \
-           not has_assignment(list(auth_client.role_assignments.list_for_scope(subnet.route_table.id)),
+           not has_assignment(auth_client.role_assignments.list_for_scope(subnet.route_table.id),
                               role_definition_id, object_id):
-            route_tables.append(subnet.route_table)
+            route_tables.add(subnet.route_table)
 
     if not route_tables:
         return
@@ -75,13 +75,6 @@ def assign_permissions_to_routetable(cli_ctx, master_subnet, worker_subnet, obje
                                              'RoleAssignmentCreateParameters', mod='models',
                                              operation_group='role_assignments')
 
-    role_definition_id = resource_id(
-        subscription=get_subscription_id(cli_ctx),
-        namespace='Microsoft.Authorization',
-        type='roleDefinitions',
-        name=DEVELOPMENT_CONTRIBUTOR if rp_mode_development() else CONTRIBUTOR,
-    )
-
     for rt in route_tables:
         role_uuid = _gen_uuid()
         auth_client.role_assignments.create(rt.id, role_uuid, RoleAssignmentCreateParameters(

python/az/aro/azext_aro/custom.py

@@ -7,7 +7,7 @@ import os
 import azext_aro.vendored_sdks.azure.mgmt.redhatopenshift.v2020_04_30.models as v2020_04_30
 
 from azext_aro._aad import AADManager
-from azext_aro._rbac import assign_contributor_to_vnet, assign_permissions_to_routetable
+from azext_aro._rbac import assign_contributor_to_vnet, assign_contributor_to_routetable
 from azext_aro._validators import validate_subnets
 from azure.cli.core.commands.client_factory import get_mgmt_service_client
 from azure.cli.core.commands.client_factory import get_subscription_id
@@ -72,9 +72,9 @@ def aro_create(cmd,  # pylint: disable=too-many-locals
 
     rp_client_sp = aad.get_service_principal(rp_client_id)
 
-    assign_contributor_to_vnet(cmd.cli_ctx, vnet, client_sp.object_id)
-    assign_contributor_to_vnet(cmd.cli_ctx, vnet, rp_client_sp.object_id)
-    assign_permissions_to_routetable(cmd.cli_ctx, master_subnet, worker_subnet, client_sp.object_id)
+    for sp_id in [client_sp.object_id, rp_client_sp.object_id]:
+        assign_contributor_to_vnet(cmd.cli_ctx, vnet, sp_id)
+        assign_contributor_to_routetable(cmd.cli_ctx, master_subnet, worker_subnet, sp_id)
 
     if rp_mode_development():
         worker_vm_size = worker_vm_size or 'Standard_D2s_v3'