Review comments resolved

src/relay/azext_relay/__init__.py

@@ -24,10 +24,8 @@ def load_command_table(self, args):
         return self.command_table
 
     def load_arguments(self, command):
-        from ._params import load_arguments_namespace, load_arguments_wcfrelay, load_arguments_hybridconnections
-        load_arguments_namespace(self, command)
-        load_arguments_wcfrelay(self, command)
-        load_arguments_hybridconnections(self, command)
+        from ._params import load_arguments_relayparams
+        load_arguments_relayparams(self, command)
 
 
 COMMAND_LOADER_CLS = RelayCommandsLoader

src/relay/azext_relay/_params.py

@@ -2,84 +2,78 @@
 # Copyright (c) Microsoft Corporation. All rights reserved.
 # Licensed under the MIT License. See License.txt in the project root for license information.
 # --------------------------------------------------------------------------------------------
-
-from azure.cli.core.commands.parameters import (tags_type, get_enum_type, resource_group_name_type)
 # pylint: disable=line-too-long
 
+from azure.cli.core.commands.parameters import (tags_type, get_enum_type, get_location_type, name_type, resource_group_name_type, get_three_state_flag)
+from knack.arguments import CLIArgumentType
+from azext_relay.relay.models.relay_management_client_enums import AccessRights, KeyType, SkuTier, Relaytype
+
+
+rights_arg_type = CLIArgumentType(options_list=['--access-rights'], nargs='+', type=str, arg_type=get_enum_type(AccessRights), help='Authorization rule rights of type list')
+key_arg_type = CLIArgumentType(options_list=['--key-name'], arg_type=get_enum_type(KeyType), help='specifies Primary or Secondary key needs to be reset')
+
 
-def load_arguments_namespace(self, _):
+def load_arguments_relayparams(self, _):
     with self.argument_context('relay') as c:
         c.argument('resource_group_name', arg_type=resource_group_name_type)
         c.argument('namespace_name', options_list=['--namespace-name'], help='name of the Namespace')
 
-    with self.argument_context('relay namespace exists') as c:
-        c.argument('namespace_name', options_list=['--name', '-n'], help='name of the Namespace')
+    with self.argument_context('relay namespace') as c:
+        c.argument('namespace_name', arg_type=name_type, help='name of the Namespace')
 
     with self.argument_context('relay namespace create') as c:
-        c.argument('namespace_name', options_list=['--name', '-n'], help='name of the Namespace')
-        c.argument('tags', options_list=['--tags', '-t'], arg_type=tags_type, help='tags for the namespace in Key value pair format')
-        c.argument('location', options_list=['--location', '-l'], help='Location')
-        c.argument('skutier', options_list=['--sku-tier'], arg_type=get_enum_type(['Basic', 'Standard']))
+        c.argument('tags', arg_type=tags_type)
+        c.argument('location', arg_type=get_location_type(self.cli_ctx))
+        c.argument('skutier', options_list=['--sku-tier'], arg_type=get_enum_type(SkuTier))
         c.argument('capacity', options_list=['--capacity'], type=int, help='Capacity for Sku')
 
-    # region Namespace Get
-    for scope in ['relay namespace show', 'relay namespace delete']:
+    for scope in ['relay namespace authorization-rule', 'relay namespace authorization-rule keys renew', 'relay namespace authorization-rule keys list']:
         with self.argument_context(scope) as c:
-            c.argument('namespace_name', options_list=['--name', '-n'], help='name of the Namespace')
+            c.argument('authorization_rule_name', arg_type=name_type, help='name of the Namespace AuthorizationRule')
+            c.argument('namespace_name', options_list=['--namespace-name'], help='name of the Namespace')
 
-    # region Namespace Authorizationrule
-    for scope in ['relay namespace authorizationrule', 'relay namespace authorizationrule keys list', 'relay namespace authorizationrule keys renew']:
-        with self.argument_context(scope) as c:
-            c.argument('authorization_rule_name', options_list=['--name', '-n'], help='name of the Namespace AuthorizationRule')
-
-    with self.argument_context('relay namespace authorizationrule create') as c:
-        c.argument('accessrights', options_list=['--access-rights'], arg_type=get_enum_type(['Send', 'Listen', 'Manage']), help='Authorization rule rights of type list')
+    with self.argument_context('relay namespace authorization-rule create') as c:
+        c.argument('accessrights', arg_type=rights_arg_type)
 
-    with self.argument_context('relay namespace authorizationrule keys renew') as c:
-        c.argument('key_type', options_list=['--key-name'], arg_type=get_enum_type(['PrimaryKey', 'SecondaryKey']), help='specifies Primary or Secondary key needs to be reset')
+    with self.argument_context('relay namespace authorization-rule keys renew') as c:
+        c.argument('key_type', arg_type=key_arg_type)
 
+# region WCF Relay
+    with self.argument_context('relay wcf-relay') as c:
+        c.argument('relay_name', arg_type=name_type, help='Name of WCF Relay')
 
-# region - WCF Relay Create
-def load_arguments_wcfrelay(self, _):
-    with self.argument_context('relay wcfrelay') as c:
-        c.argument('relay_name', options_list=['--name', '-n'], help='Name of WCF Relay')
+    with self.argument_context('relay wcf-relay create') as c:
+        c.argument('relay_type', options_list=['--relay-type'], arg_type=get_enum_type(Relaytype), help='WCF relay type.')
+        c.argument('requires_client_authorization', options_list=['--requires-client-authorization'], arg_type=get_three_state_flag(), help='True if client authorization is needed for this relay; otherwise, false.')
+        c.argument('requires_transport_security', options_list=['--requires-transport-security'], arg_type=get_three_state_flag(), help='True if transport security is needed for this relay; otherwise, false.')
+        c.argument('user_metadata', help='The usermetadata is a placeholder to store user-defined string data for the WCF Relay endpoint. For example, it can be used to store descriptive data, such as list of teams and their contact information. Also, user-defined configuration settings can be stored.')
 
-    with self.argument_context('relay wcfrelay create') as c:
-        c.argument('relay_type', options_list=['--relay-type'], arg_type=get_enum_type(['NetTcp', 'Http']), help='WCF relay type.')
-        c.argument('requires_client_authorization', options_list=['--requires-client-authorization'], type=bool, help='True if client authorization is needed for this relay; otherwise, false.')
-        c.argument('requires_transport_security', options_list=['--requires-transport-security'], type=bool, help='True if transport security is needed for this relay; otherwise, false.')
-        c.argument('user_metadata', options_list=['--user-metadata'], help='The usermetadata is a placeholder to store user-defined string data for the WCF Relay endpoint. For example, it can be used to store descriptive data, such as list of teams and their contact information. Also, user-defined configuration settings can be stored.')
-
-    # region WCF Relay Authorizationrule
-    for scope in ['relay wcfrelay authorizationrule', 'relay wcfrelay authorizationrule keys list', 'relay wcfrelay authorizationrule keys renew']:
+    for scope in ['relay wcf-relay authorization-rule', 'relay wcf-relay authorization-rule keys list', 'relay wcf-relay authorization-rule keys renew']:
         with self.argument_context(scope) as c:
-            c.argument('authorization_rule_name', options_list=['--name', '-n'], help='name of the WCF Relay AuthorizationRule')
+            c.argument('authorization_rule_name', arg_type=name_type, help='name of the WCF Relay AuthorizationRule')
             c.argument('relay_name', options_list=['--wcfrelay-name'], help='name of the WCF Relay')
 
-    with self.argument_context('relay wcfrelay authorizationrule create') as c:
-        c.argument('rights', options_list=['--access-rights'], arg_type=get_enum_type(['Send', 'Listen', 'Manage']), help='AuthorizationRule rights of type list')
-
-    with self.argument_context('relay wcfrelay authorizationrule keys renew') as c:
-        c.argument('key_type', options_list=['--key-name'], arg_type=get_enum_type(['PrimaryKey', 'SecondaryKey']))
+    with self.argument_context('relay wcf-relay authorization-rule create') as c:
+        c.argument('rights', arg_type=rights_arg_type)
 
+    with self.argument_context('relay wcf-relay authorization-rule keys renew') as c:
+        c.argument('key_type', arg_type=key_arg_type)
 
-# region - Hybrid Connection Create
-def load_arguments_hybridconnections(self, _):
+# region Hybrid Connection
     with self.argument_context('relay hybrid-connections') as c:
-        c.argument('hybrid_connection_name', options_list=['--name', '-n'], help='Name of Hybrid Connection')
+        c.argument('hybrid_connection_name', arg_type=name_type, help='Name of Hybrid Connection')
 
     with self.argument_context('relay hybrid-connections create') as c:
-        c.argument('requires_client_authorization', options_list=['--requires-client-authorization'], type=bool, help='True if client authorization is needed for this relay; otherwise, false.')
-        c.argument('user_metadata', options_list=['--user-metadata'], help='The usermetadata is a placeholder to store user-defined string data for the Hybrid Connection endpoint. For example, it can be used to store descriptive data, such as list of teams and their contact information. Also, user-defined configuration settings can be stored.')
+        c.argument('requires_client_authorization', arg_type=get_three_state_flag(), help='True if client authorization is needed for this relay; otherwise, false.')
+        c.argument('user_metadata', help='The usermetadata is a placeholder to store user-defined string data for the Hybrid Connection endpoint. For example, it can be used to store descriptive data, such as list of teams and their contact information. Also, user-defined configuration settings can be stored.')
 
-    # region Hybrid Connection Authorizationrule
-    for scope in ['relay hybrid-connections authorizationrule', 'relay hybrid-connections authorizationrule keys list', 'relay Hybrid Connection authorizationrule keys renew']:
+    for scope in ['relay hybrid-connections authorization-rule', 'relay hybrid-connections authorization-rule keys list', 'relay Hybrid Connection authorization-rule keys renew']:
         with self.argument_context(scope) as c:
-            c.argument('authorization_rule_name', options_list=['--name', '-n'], help='name of the Hybrid Connection AuthorizationRule')
+            c.argument('authorization_rule_name', arg_type=name_type, help='name of the Hybrid Connection AuthorizationRule')
             c.argument('hybrid_connection_name', options_list=['--hybrid-connection-name'], help='name of the Hybrid Connection')
 
-    with self.argument_context('relay hybrid-connections authorizationrule create') as c:
-        c.argument('rights', options_list=['--access-rights'], arg_type=get_enum_type(['Send', 'Listen', 'Manage']), help='AuthorizationRule rights of type list')
+    with self.argument_context('relay hybrid-connections authorization-rule create') as c:
+        c.argument('rights', arg_type=rights_arg_type)
 
-    with self.argument_context('relay hybrid-connections authorizationrule keys renew') as c:
-        c.argument('key_type', options_list=['--key-name'], arg_type=get_enum_type(['PrimaryKey', 'SecondaryKey']))
+    with self.argument_context('relay hybrid-connections authorization-rule keys renew') as c:
+        c.argument('key_type', arg_type=key_arg_type)

src/relay/azext_relay/commands.py

@@ -39,7 +39,7 @@ def load_command_table(self, _):
         g.command('delete', 'delete')
         g.command('exists', 'check_name_availability_method')
 
-    with self.command_group('relay namespace authorizationrule', relay_namespace_util, client_factory=namespaces_mgmt_client_factory) as g:
+    with self.command_group('relay namespace authorization-rule', relay_namespace_util, client_factory=namespaces_mgmt_client_factory) as g:
         g.custom_command('create', 'cli_namespaceautho_create')
         g.command('show', 'get_authorization_rule', exception_handler=empty_on_404)
         g.command('list', 'list_authorization_rules', exception_handler=empty_on_404)
@@ -48,13 +48,13 @@ def load_command_table(self, _):
         g.command('delete', 'delete_authorization_rule')
 
 # WcfRelay Region
-    with self.command_group('relay wcfrelay', relay_wcf_relay_util, client_factory=wcfrelay_mgmt_client_factory) as g:
+    with self.command_group('relay wcf-relay', relay_wcf_relay_util, client_factory=wcfrelay_mgmt_client_factory) as g:
         g.custom_command('create', 'cli_wcfrelay_create')
         g.command('show', 'get', exception_handler=empty_on_404)
         g.command('list', 'list_by_namespace', exception_handler=empty_on_404)
         g.command('delete', 'delete')
 
-    with self.command_group('relay wcfrelay authorizationrule', relay_wcf_relay_util, client_factory=wcfrelay_mgmt_client_factory) as g:
+    with self.command_group('relay wcf-relay authorization-rule', relay_wcf_relay_util, client_factory=wcfrelay_mgmt_client_factory) as g:
         g.custom_command('create', 'cli_wcfrelayautho_create')
         g.command('show', 'get_authorization_rule', exception_handler=empty_on_404)
         g.command('list', 'list_authorization_rules', exception_handler=empty_on_404)
@@ -69,7 +69,7 @@ def load_command_table(self, _):
         g.command('list', 'list_by_namespace', exception_handler=empty_on_404)
         g.command('delete', 'delete')
 
-    with self.command_group('relay hybrid-connections authorizationrule', relay_hybrid_connections_util, client_factory=hydrid_connections_mgmt_client_factory) as g:
+    with self.command_group('relay hybrid-connections authorization-rule', relay_hybrid_connections_util, client_factory=hydrid_connections_mgmt_client_factory) as g:
         g.custom_command('create', 'cli_hybridconnectionsautho_create')
         g.command('show', 'get_authorization_rule', exception_handler=empty_on_404)
         g.command('list', 'list_authorization_rules', exception_handler=empty_on_404)

src/relay/azext_relay/custom.py

@@ -7,14 +7,15 @@
 # pylint: disable=too-many-lines
 # pylint: disable=inconsistent-return-statements
 
-from azext_relay._utils import accessrights_converter
-
-from azext_relay.relay.models import (RelayNamespace, Sku, WcfRelay)
-
 
 # Namespace Region
 def cli_namespace_create(client, resource_group_name, namespace_name, location, tags=None, skutier=None):
-    return client.create_or_update(resource_group_name, namespace_name, RelayNamespace(location, tags, Sku(skutier)))
+    from azext_relay.relay.models import RelayNamespace, Sku
+    return client.create_or_update(
+        resource_group_name=resource_group_name,
+        namespace_name=namespace_name,
+        parameters=RelayNamespace(location, tags, Sku(skutier))
+    )
 
 
 def cli_namespace_list(client, resource_group_name=None):
@@ -27,13 +28,21 @@ def cli_namespace_list(client, resource_group_name=None):
 
 
 # Namespace Authorization rule:
-def cli_namespaceautho_create(client, resource_group_name, namespace_name, name, accessrights=None):
-    return client.create_or_update_authorization_rule(resource_group_name, namespace_name, name,
-                                                      accessrights_converter(accessrights))
+def cli_namespaceautho_create(client, resource_group_name, namespace_name, name, access_rights=None):
+    from azext_relay._utils import accessrights_converter
+    if not access_rights:
+        access_rights = access_rights.split()
+    return client.create_or_update_authorization_rule(
+        resource_group_name=resource_group_name,
+        namespace_name=namespace_name,
+        authorization_rule_name=name,
+        rights=accessrights_converter(access_rights)
+    )
 
 
 # WcfRelay Region
 def cli_wcfrelay_create(client, resource_group_name, namespace_name, name, relay_type=None, requires_client_authorization=None, requires_transport_security=None, user_metadata=None):
+    from azext_relay.relay.models import WcfRelay
     wcfrelayparameter1 = WcfRelay()
     if relay_type:
         wcfrelayparameter1.relay_type = relay_type
@@ -47,17 +56,34 @@ def cli_wcfrelay_create(client, resource_group_name, namespace_name, name, relay
     if user_metadata:
         wcfrelayparameter1.user_metadata = user_metadata
 
-    return client.create_or_update(resource_group_name, namespace_name, name, wcfrelayparameter1)
+    return client.create_or_update(
+        resource_group_name=resource_group_name,
+        namespace_name=namespace_name,
+        relay_name=name,
+        parameters=wcfrelayparameter1
+    )
 
 
 def cli_wcfrelayautho_create(client, resource_group_name, namespace_name, wcfrelay_name, name, access_rights=None):
-    return client.create_or_update_authorization_rule(resource_group_name, namespace_name, wcfrelay_name, name,
-                                                      accessrights_converter(access_rights))
+    from azext_relay._utils import accessrights_converter
+    return client.create_or_update_authorization_rule(
+        resource_group_name=resource_group_name,
+        namespace_name=namespace_name,
+        relay_name=wcfrelay_name,
+        authorization_rule_name=name,
+        rights=accessrights_converter(access_rights)
+    )
 
 
 def cli_hybridconnectionsautho_create(client, resource_group_name, namespace_name, hybrid_connection_name, name, access_rights=None):
-    return client.create_or_update_authorization_rule(resource_group_name, namespace_name, hybrid_connection_name, name,
-                                                      accessrights_converter(access_rights))
+    from azext_relay._utils import accessrights_converter
+    return client.create_or_update_authorization_rule(
+        resource_group_name=resource_group_name,
+        namespace_name=namespace_name,
+        hybrid_connection_name=hybrid_connection_name,
+        authorization_rule_name=name,
+        rights=accessrights_converter(access_rights)
+    )
 
 
 # pylint: disable=inconsistent-return-statements