Fix long CLI error messages

The python-octaviaclient plugin was using "choices" for validating
port numbers and weights. The cliff "choices" parameter will output
an error that includes every valid "choice". For port numbers, this
is 65,000+ valid options.
This patch changes this to produce a more user friendly error
message.

This patch also fixes the listener protocol-port type validation.

Change-Id: I4cb32bcc4681bd99f178d31547af37a7c582d210
Story: 2007218
Task: 38470
(cherry picked from commit cc3e82d5de)

octaviaclient/osc/v2/constants.py

@@ -312,3 +312,11 @@ FLAVORPROFILE_COLUMNS = (
     'name',
     'provider_name',
 )
+
+# TCP/UDP port min/max
+MIN_PORT_NUMBER = 1
+MAX_PORT_NUMBER = 65535
+
+# Member weight min/max
+MIN_WEIGHT = 0
+MAX_WEIGHT = 256

octaviaclient/osc/v2/listener.py

@@ -21,6 +21,7 @@ from osc_lib import utils
 
 from octaviaclient.osc.v2 import constants as const
 from octaviaclient.osc.v2 import utils as v2_utils
+from octaviaclient.osc.v2 import validate
 
 PROTOCOL_CHOICES = ['TCP', 'HTTP', 'HTTPS', 'TERMINATED_HTTPS', 'UDP']
 CLIENT_AUTH_CHOICES = ['NONE', 'OPTIONAL', 'MANDATORY']
@@ -93,6 +94,7 @@ class CreateListener(command.ShowOne):
             '--protocol-port',
             metavar='<port>',
             required=True,
+            type=int,
             help="Set the protocol port number for the listener."
         )
         parser.add_argument(
@@ -163,6 +165,9 @@ class CreateListener(command.ShowOne):
         rows = const.LISTENER_ROWS
         attrs = v2_utils.get_listener_attrs(self.app.client_manager,
                                             parsed_args)
+
+        validate.check_listener_attrs(attrs)
+
         body = {"listener": attrs}
         data = self.app.client_manager.load_balancer.listener_create(
             json=body)

octaviaclient/osc/v2/member.py

@@ -20,6 +20,7 @@ from osc_lib import utils
 
 from octaviaclient.osc.v2 import constants as const
 from octaviaclient.osc.v2 import utils as v2_utils
+from octaviaclient.osc.v2 import validate
 
 
 class ListMember(lister.Lister):
@@ -118,7 +119,6 @@ class CreateMember(command.ShowOne):
             '--weight',
             metavar='<weight>',
             type=int,
-            choices=range(0, 256),
             help="The weight of a member determines the portion of requests "
                  "or connections it services compared to the other members of "
                  "the pool."
@@ -138,7 +138,6 @@ class CreateMember(command.ShowOne):
             '--protocol-port',
             metavar='<protocol_port>',
             type=int,
-            choices=range(1, 65535),
             help="The protocol port number the backend member server is "
                  "listening on.",
             required=True
@@ -147,7 +146,6 @@ class CreateMember(command.ShowOne):
             '--monitor-port',
             metavar='<monitor_port>',
             type=int,
-            choices=range(1, 65535),
             help="An alternate protocol port used for health monitoring a "
                  "backend member.",
         )
@@ -176,6 +174,9 @@ class CreateMember(command.ShowOne):
     def take_action(self, parsed_args):
         rows = const.MEMBER_ROWS
         attrs = v2_utils.get_member_attrs(self.app.client_manager, parsed_args)
+
+        validate.check_member_attrs(attrs)
+
         pool_id = attrs.pop('pool_id')
 
         body = {"member": attrs}
@@ -227,14 +228,12 @@ class SetMember(command.Command):
             '--weight',
             metavar='<weight>',
             type=int,
-            choices=range(0, 256),
             help="Set the weight of member in the pool"
         )
         parser.add_argument(
             '--monitor-port',
             metavar='<monitor_port>',
             type=int,
-            choices=range(1, 65535),
             help="An alternate protocol port used for health monitoring a "
                  "backend member",
         )
@@ -261,6 +260,9 @@ class SetMember(command.Command):
 
     def take_action(self, parsed_args):
         attrs = v2_utils.get_member_attrs(self.app.client_manager, parsed_args)
+
+        validate.check_member_attrs(attrs)
+
         pool_id = attrs.pop('pool_id')
         member_id = attrs.pop('member_id')
         post_data = {"member": attrs}

octaviaclient/osc/v2/validate.py

@@ -13,6 +13,8 @@
 
 from osc_lib import exceptions
 
+from octaviaclient.osc.v2 import constants
+
 
 def check_l7policy_attrs(attrs):
     msg = None
@@ -39,3 +41,39 @@ def check_l7rule_attrs(attrs):
                     "Missing argument: --type {type_name} requires "
                     "--key <key>".format(type_name=attrs['type']))
                 raise exceptions.CommandError(msg)
+
+
+# Handling these range validations here instead of "choices" as "choices" will
+# output every possible option in the error message.
+def _validate_TCPUDP_port_range(port_number, parameter_name):
+    if (port_number < constants.MIN_PORT_NUMBER or
+            port_number > constants.MAX_PORT_NUMBER):
+        msg = ("Invalid input for field/attribute '{name}', Value: "
+               "'{port}'. Value must be between {pmin} and {pmax}.".format(
+                   name=parameter_name, port=port_number,
+                   pmin=constants.MIN_PORT_NUMBER,
+                   pmax=constants.MAX_PORT_NUMBER))
+        raise exceptions.InvalidValue(msg)
+
+
+def check_listener_attrs(attrs):
+    if 'protocol_port' in attrs:
+        _validate_TCPUDP_port_range(attrs['protocol_port'], 'protocol-port')
+
+
+def check_member_attrs(attrs):
+    if 'protocol_port' in attrs:
+        _validate_TCPUDP_port_range(attrs['protocol_port'], 'protocol-port')
+
+    if 'member_port' in attrs:
+        _validate_TCPUDP_port_range(attrs['member_port'], 'member-port')
+
+    if 'weight' in attrs:
+        if(attrs['weight'] < constants.MIN_WEIGHT or
+           attrs['weight'] > constants.MAX_WEIGHT):
+            msg = ("Invalid input for field/attribute 'weight', Value: "
+                   "'{weight}'. Value must be between {wmin} and "
+                   "{wmax}.".format(weight=attrs['weight'],
+                                    wmin=constants.MIN_WEIGHT,
+                                    wmax=constants.MAX_WEIGHT))
+            raise exceptions.InvalidValue(msg)

octaviaclient/tests/unit/osc/v2/test_listener.py

@@ -119,7 +119,7 @@ class TestListenerCreate(TestListener):
             ('loadbalancer', 'mock_lb_id'),
             ('name', self._listener.name),
             ('protocol', 'HTTP'),
-            ('protocol_port', '80')
+            ('protocol_port', 80)
         ]
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
@@ -149,7 +149,7 @@ class TestListenerCreate(TestListener):
             ('loadbalancer', 'mock_lb_id'),
             ('name', self._listener.name),
             ('protocol', 'TERMINATED_HTTPS'),
-            ('protocol_port', '443'),
+            ('protocol_port', 443),
             ('sni_container_refs', self._listener.sni_container_refs),
             ('default_tls_container_ref',
              self._listener.default_tls_container_ref),
@@ -180,7 +180,7 @@ class TestListenerCreate(TestListener):
             ('loadbalancer', 'mock_lb_id'),
             ('name', self._listener.name),
             ('protocol', 'HTTP'),
-            ('protocol_port', '80'),
+            ('protocol_port', 80),
             ('timeout_client_data', 123),
             ('timeout_member_connect', 234),
             ('timeout_member_data', 345),

octaviaclient/tests/unit/osc/v2/test_validations.py

@@ -14,6 +14,7 @@
 from osc_lib import exceptions
 from osc_lib.tests import utils
 
+from octaviaclient.osc.v2 import constants
 from octaviaclient.osc.v2 import validate
 
 
@@ -75,3 +76,80 @@ class TestValidations(utils.TestCommand):
             self.assertRaises(
                 exceptions.CommandError,
                 validate.check_l7rule_attrs, attrs_dict)
+
+    def test_validate_TCPUDP_port_range(self):
+        # Positive tests, should not raise
+        validate._validate_TCPUDP_port_range(constants.MIN_PORT_NUMBER,
+                                             "fake-parameter")
+        validate._validate_TCPUDP_port_range(constants.MAX_PORT_NUMBER,
+                                             "fake-parameter")
+
+        # Negative tests, should raise
+        self.assertRaises(exceptions.InvalidValue,
+                          validate._validate_TCPUDP_port_range,
+                          constants.MIN_PORT_NUMBER - 1, "fake-parameter")
+        self.assertRaises(exceptions.InvalidValue,
+                          validate._validate_TCPUDP_port_range,
+                          constants.MAX_PORT_NUMBER + 1, "fake-parameter")
+
+    def test_check_listener_attrs(self):
+        # Positive tests, should not raise
+        attrs_dict = {'protocol_port': constants.MIN_PORT_NUMBER}
+        validate.check_listener_attrs(attrs_dict)
+        attrs_dict = {'protocol_port': constants.MAX_PORT_NUMBER}
+        validate.check_listener_attrs(attrs_dict)
+
+        # Negative tests, should raise
+        attrs_dict = {'protocol_port': constants.MIN_PORT_NUMBER - 1}
+        self.assertRaises(exceptions.InvalidValue,
+                          validate.check_listener_attrs, attrs_dict)
+        attrs_dict = {'protocol_port': constants.MAX_PORT_NUMBER + 1}
+        self.assertRaises(exceptions.InvalidValue,
+                          validate.check_listener_attrs, attrs_dict)
+
+    def test_check_member_attrs(self):
+        # Positive tests, should not raise
+        attrs_dict = {'protocol_port': constants.MIN_PORT_NUMBER,
+                      'member_port': constants.MIN_PORT_NUMBER,
+                      'weight': constants.MIN_WEIGHT}
+        validate.check_member_attrs(attrs_dict)
+        attrs_dict = {'protocol_port': constants.MAX_PORT_NUMBER,
+                      'member_port': constants.MAX_PORT_NUMBER,
+                      'weight': constants.MAX_WEIGHT}
+        validate.check_member_attrs(attrs_dict)
+
+        # Negative tests, should raise - protocol-port
+        attrs_dict = {'protocol_port': constants.MIN_PORT_NUMBER - 1,
+                      'member_port': constants.MIN_PORT_NUMBER,
+                      'weight': constants.MIN_WEIGHT}
+        self.assertRaises(exceptions.InvalidValue, validate.check_member_attrs,
+                          attrs_dict)
+        attrs_dict = {'protocol_port': constants.MAX_PORT_NUMBER + 1,
+                      'member_port': constants.MIN_PORT_NUMBER,
+                      'weight': constants.MIN_WEIGHT}
+        self.assertRaises(exceptions.InvalidValue, validate.check_member_attrs,
+                          attrs_dict)
+
+        # Negative tests, should raise - member-port
+        attrs_dict = {'protocol_port': constants.MIN_PORT_NUMBER,
+                      'member_port': constants.MIN_PORT_NUMBER - 1,
+                      'weight': constants.MIN_WEIGHT}
+        self.assertRaises(exceptions.InvalidValue, validate.check_member_attrs,
+                          attrs_dict)
+        attrs_dict = {'protocol_port': constants.MIN_PORT_NUMBER,
+                      'member_port': constants.MAX_PORT_NUMBER + 1,
+                      'weight': constants.MIN_WEIGHT}
+        self.assertRaises(exceptions.InvalidValue, validate.check_member_attrs,
+                          attrs_dict)
+
+        # Negative tests, should raise - weight
+        attrs_dict = {'protocol_port': constants.MIN_PORT_NUMBER,
+                      'member_port': constants.MIN_PORT_NUMBER,
+                      'weight': constants.MIN_WEIGHT - 1}
+        self.assertRaises(exceptions.InvalidValue, validate.check_member_attrs,
+                          attrs_dict)
+        attrs_dict = {'protocol_port': constants.MIN_PORT_NUMBER,
+                      'member_port': constants.MIN_PORT_NUMBER,
+                      'weight': constants.MAX_WEIGHT + 1}
+        self.assertRaises(exceptions.InvalidValue, validate.check_member_attrs,
+                          attrs_dict)