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

5 files changed, 30 insertions, 43 deletions

diff --git a/trove/common/cfg.py b/trove/common/cfg.py
index e9e3dd17..e5df2d60 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 4efe4771..c0da17ac 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 5a8c8bb4..d28cd185 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 5aa1f2ef..9f4b7c52 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 7fbba5dd..59e7793d 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(