From 91d443e06b50143a707235363bff9ab97d68a8f4 Mon Sep 17 00:00:00 2001 From: Trevor McCasland Date: Mon, 31 Oct 2016 13:19:47 -0500 Subject: [PATCH] Improve list-of-ports validation List options tcp_ports and udp_ports are lists of strings and some with a '-' in the middle to indicate a range. To help validate the options better a new type was introduced to oslo.config called Range. oslo.config version 3.18.0 merged this Range type which will no longer require the following in our project: * utility function gen_ports because a Range of ints are returned * test to check for proper from-to format, the type is smart so it flips the numbers around for us. So 63000-300 returns Range(300, 63001) 63001 because inclusion=True is set by default. Change-Id: I63b6a865a3f3c79202dd299f6cd25dd59e182252 Closes-Bug: #1500141 --- trove/common/cfg.py | 45 +++++++++++-------- trove/common/utils.py | 10 ----- trove/extensions/security_group/service.py | 5 +-- trove/taskmanager/models.py | 4 +- .../unittests/taskmanager/test_models.py | 9 ---- 5 files changed, 30 insertions(+), 43 deletions(-) diff --git a/trove/common/cfg.py b/trove/common/cfg.py index e9e3dd17bd..e5df2d6085 100644 --- a/trove/common/cfg.py +++ b/trove/common/cfg.py @@ -19,6 +19,7 @@ import os.path from oslo_config import cfg from oslo_config.cfg import NoSuchOptError +from oslo_config import types from oslo_log import log as logging from oslo_middleware import cors from osprofiler import opts as profiler @@ -26,6 +27,7 @@ from osprofiler import opts as profiler from trove.common.i18n import _ from trove.version import version_info as version +ListOfPortsType = types.Range(1, 65535) LOG = logging.getLogger(__name__) UNKNOWN_SERVICE_ID = 'unknown-service-id-error' @@ -509,11 +511,11 @@ mysql_group = cfg.OptGroup( mysql_opts = [ cfg.BoolOpt('icmp', default=False, help='Whether to permit ICMP.'), - cfg.ListOpt('tcp_ports', default=["3306"], + cfg.ListOpt('tcp_ports', default=["3306"], item_type=ListOfPortsType, help='List of TCP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), - cfg.ListOpt('udp_ports', default=[], + cfg.ListOpt('udp_ports', default=[], item_type=ListOfPortsType, help='List of UDP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), @@ -592,11 +594,11 @@ percona_group = cfg.OptGroup( percona_opts = [ cfg.BoolOpt('icmp', default=False, help='Whether to permit ICMP.'), - cfg.ListOpt('tcp_ports', default=["3306"], + cfg.ListOpt('tcp_ports', default=["3306"], item_type=ListOfPortsType, help='List of TCP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), - cfg.ListOpt('udp_ports', default=[], + cfg.ListOpt('udp_ports', default=[], item_type=ListOfPortsType, help='List of UDP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), @@ -681,10 +683,11 @@ pxc_opts = [ cfg.BoolOpt('icmp', default=False, help='Whether to permit ICMP.'), cfg.ListOpt('tcp_ports', default=["3306", "4444", "4567", "4568"], + item_type=ListOfPortsType, help='List of TCP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), - cfg.ListOpt('udp_ports', default=[], + cfg.ListOpt('udp_ports', default=[], item_type=ListOfPortsType, help='List of UDP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), @@ -774,10 +777,11 @@ redis_opts = [ cfg.BoolOpt('icmp', default=False, help='Whether to permit ICMP.'), cfg.ListOpt('tcp_ports', default=["6379", "16379"], + item_type=ListOfPortsType, help='List of TCP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), - cfg.ListOpt('udp_ports', default=[], + cfg.ListOpt('udp_ports', default=[], item_type=ListOfPortsType, help='List of UDP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), @@ -851,10 +855,11 @@ cassandra_opts = [ cfg.BoolOpt('icmp', default=False, help='Whether to permit ICMP.'), cfg.ListOpt('tcp_ports', default=["7000", "7001", "7199", "9042", "9160"], + item_type=ListOfPortsType, help='List of TCP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), - cfg.ListOpt('udp_ports', default=[], + cfg.ListOpt('udp_ports', default=[], item_type=ListOfPortsType, help='List of UDP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), @@ -953,13 +958,13 @@ couchbase_group = cfg.OptGroup( couchbase_opts = [ cfg.BoolOpt('icmp', default=False, help='Whether to permit ICMP.'), - cfg.ListOpt('tcp_ports', + cfg.ListOpt('tcp_ports', item_type=ListOfPortsType, default=["8091", "8092", "4369", "11209-11211", "21100-21199"], help='List of TCP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), - cfg.ListOpt('udp_ports', default=[], + cfg.ListOpt('udp_ports', default=[], item_type=ListOfPortsType, help='List of UDP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), @@ -1018,10 +1023,11 @@ mongodb_opts = [ cfg.BoolOpt('icmp', default=False, help='Whether to permit ICMP.'), cfg.ListOpt('tcp_ports', default=["2500", "27017", "27019"], + item_type=ListOfPortsType, help='List of TCP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), - cfg.ListOpt('udp_ports', default=[], + cfg.ListOpt('udp_ports', default=[], item_type=ListOfPortsType, help='List of UDP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), @@ -1110,11 +1116,11 @@ postgresql_group = cfg.OptGroup( postgresql_opts = [ cfg.BoolOpt('icmp', default=False, help='Whether to permit ICMP.'), - cfg.ListOpt('tcp_ports', default=["5432"], + cfg.ListOpt('tcp_ports', default=["5432"], item_type=ListOfPortsType, help='List of TCP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), - cfg.ListOpt('udp_ports', default=[], + cfg.ListOpt('udp_ports', default=[], item_type=ListOfPortsType, help='List of UDP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), @@ -1187,11 +1193,11 @@ couchdb_opts = [ cfg.BoolOpt('icmp', default=False, help='Whether to permit ICMP.'), cfg.ListOpt('tcp_ports', - default=["5984"], + default=["5984"], item_type=ListOfPortsType, help='List of TCP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), - cfg.ListOpt('udp_ports', default=[], + cfg.ListOpt('udp_ports', default=[], item_type=ListOfPortsType, help='List of UDP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), @@ -1248,12 +1254,12 @@ vertica_group = cfg.OptGroup( vertica_opts = [ cfg.BoolOpt('icmp', default=False, help='Whether to permit ICMP.'), - cfg.ListOpt('tcp_ports', + cfg.ListOpt('tcp_ports', item_type=ListOfPortsType, default=["5433", "5434", "22", "5444", "5450", "4803"], help='List of TCP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), - cfg.ListOpt('udp_ports', + cfg.ListOpt('udp_ports', item_type=ListOfPortsType, default=["5433", "4803", "4804", "6453"], help='List of UDP ports and/or port ranges to open ' 'in the security group (only applicable ' @@ -1319,11 +1325,11 @@ db2_opts = [ cfg.BoolOpt('icmp', default=False, help='Whether to permit ICMP.'), cfg.ListOpt('tcp_ports', - default=["50000"], + default=["50000"], item_type=ListOfPortsType, help='List of TCP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), - cfg.ListOpt('udp_ports', default=[], + cfg.ListOpt('udp_ports', default=[], item_type=ListOfPortsType, help='List of UDP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), @@ -1379,10 +1385,11 @@ mariadb_opts = [ cfg.BoolOpt('icmp', default=False, help='Whether to permit ICMP.'), cfg.ListOpt('tcp_ports', default=["3306", "4444", "4567", "4568"], + item_type=ListOfPortsType, help='List of TCP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), - cfg.ListOpt('udp_ports', default=[], + cfg.ListOpt('udp_ports', default=[], item_type=ListOfPortsType, help='List of UDP ports and/or port ranges to open ' 'in the security group (only applicable ' 'if trove_security_groups_support is True).'), diff --git a/trove/common/utils.py b/trove/common/utils.py index 4efe4771b5..c0da17acda 100644 --- a/trove/common/utils.py +++ b/trove/common/utils.py @@ -307,16 +307,6 @@ def try_recover(func): return _decorator -def gen_ports(portstr): - from_port, sep, to_port = portstr.partition('-') - if not (to_port and from_port): - if not sep: - to_port = from_port - if int(from_port) > int(to_port): - raise ValueError - return from_port, to_port - - def unpack_singleton(container): """Unpack singleton collections. diff --git a/trove/extensions/security_group/service.py b/trove/extensions/security_group/service.py index 5a8c8bb43f..d28cd18526 100644 --- a/trove/extensions/security_group/service.py +++ b/trove/extensions/security_group/service.py @@ -19,7 +19,6 @@ from oslo_log import log as logging from trove.common import cfg from trove.common import exception from trove.common.i18n import _ -from trove.common import utils from trove.common import wsgi from trove.datastore.models import DatastoreVersion from trove.extensions.security_group import models @@ -103,9 +102,9 @@ class SecurityGroupRuleController(wsgi.Controller): rules = [] try: for port_or_range in set(ports): - from_, to_ = utils.gen_ports(port_or_range) + from_, to_ = port_or_range[0], port_or_range[-1] rule = models.SecurityGroupRule.create_sec_group_rule( - sec_group, protocol, int(from_), int(to_), + sec_group, protocol, from_, to_, body['security_group_rule']['cidr'], context, CONF.os_region_name) rules.append(rule) diff --git a/trove/taskmanager/models.py b/trove/taskmanager/models.py index 5aa1f2eff7..9f4b7c5257 100755 --- a/trove/taskmanager/models.py +++ b/trove/taskmanager/models.py @@ -761,7 +761,7 @@ class FreshInstanceTasks(FreshInstance, NotifyMixin, ConfigurationMixin): final = [] cidr = CONF.trove_security_group_rule_cidr for port_or_range in set(rule_ports): - from_, to_ = utils.gen_ports(port_or_range) + from_, to_ = port_or_range[0], port_or_range[-1] final.append({'cidr': cidr, 'from_': str(from_), 'to_': str(to_)}) @@ -1014,7 +1014,7 @@ class FreshInstanceTasks(FreshInstance, NotifyMixin, ConfigurationMixin): for port_or_range in set(ports): try: from_, to_ = (None, None) - from_, to_ = utils.gen_ports(port_or_range) + from_, to_ = port_or_range[0], port_or_range[-1] SecurityGroupRule.create_sec_group_rule( s_group, protocol, int(from_), int(to_), cidr, self.context, self.region_name) diff --git a/trove/tests/unittests/taskmanager/test_models.py b/trove/tests/unittests/taskmanager/test_models.py index 7fbba5dd20..59e7793d75 100644 --- a/trove/tests/unittests/taskmanager/test_models.py +++ b/trove/tests/unittests/taskmanager/test_models.py @@ -352,15 +352,6 @@ class FreshInstanceTasksTest(trove_testtools.TestCase): self.freshinstancetasks._create_secgroup, datastore_manager) - def test_create_sg_rules_greater_than_exception_raised(self): - datastore_manager = 'mysql' - self.task_models_conf_mock.get = Mock( - return_value=FakeOptGroup(tcp_ports=['3306', '33060-3306'])) - self.freshinstancetasks.update_db = Mock() - self.assertRaises(MalformedSecurityGroupRuleError, - self.freshinstancetasks._create_secgroup, - datastore_manager) - def test_create_sg_rules_success_with_duplicated_port_or_range(self): datastore_manager = 'mysql' self.task_models_conf_mock.get = Mock(