openstack
/
python-octaviaclient
Code Issues Proposed changes

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)
This commit is contained in:
Michael Johnson 2020-01-27 08:44:26 -08:00
parent 3e83788739
commit b5397ea3e4
6 changed files with 139 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
octaviaclient/osc/v2/constants.py
View File

@ -313,3 +313,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

5
octaviaclient/osc/v2/listener.py
View File

@ -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(
@ -173,6 +175,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)

12
octaviaclient/osc/v2/member.py
View File

@ -22,6 +22,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):
@ -120,7 +121,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."
@ -140,7 +140,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
@ -149,7 +148,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.",
)
@ -178,6 +176,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}
@ -229,14 +230,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",
)
@ -263,6 +262,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}

38
octaviaclient/osc/v2/validate.py
View File

@ -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)

6
octaviaclient/tests/unit/osc/v2/test_listener.py
View File

@ -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),

78
octaviaclient/tests/unit/osc/v2/test_validations.py
View File

@ -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)