]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Kill hostname validation for subnet:dns_nameservers
authorIhar Hrachyshka <ihrachys@redhat.com>
Mon, 30 Mar 2015 12:35:15 +0000 (14:35 +0200)
committerIhar Hrachyshka <ihrachys@redhat.com>
Fri, 10 Apr 2015 17:11:58 +0000 (19:11 +0200)
DNS servers that are hostnames seem like a bad idea. They are also not
supported by base_db_plugin_v2 [1] anyway, so there is no big reason to
pass them thru API validation only to receive InvalidInput later inside
plugin code.

[1]: http://git.openstack.org/cgit/openstack/neutron/tree/neutron/db/db_base_plugin_v2.py#n1049

Change-Id: I2db00fe266fe0748d0e6327fbad22fa16b751da8
Related-Bug: #1396932

neutron/api/v2/attributes.py
neutron/tests/unit/api/v2/test_attributes.py

index 61f4d87a9ac68fc26bf272c1a2867b8eee5ed6b6..1f992a0c18192e6037df780d46d0511ecab8e532 100644 (file)
@@ -259,20 +259,6 @@ def _validate_fixed_ips(data, valid_values=None):
                 return msg
 
 
-def _validate_ip_or_hostname(host):
-    ip_err = _validate_ip_address(host)
-    if not ip_err:
-        return
-    name_err = _validate_hostname(host)
-    if not name_err:
-        return
-    msg = _("%(host)s is not a valid IP or hostname. Details: "
-            "%(ip_err)s, %(name_err)s") % {'ip_err': ip_err, 'host': host,
-                                           'name_err': name_err}
-    LOG.debug(msg)
-    return msg
-
-
 def _validate_nameservers(data, valid_values=None):
     if not hasattr(data, '__iter__'):
         msg = _("Invalid data format for nameserver: '%s'") % data
@@ -281,8 +267,8 @@ def _validate_nameservers(data, valid_values=None):
 
     hosts = []
     for host in data:
-        # This may be an IP or a hostname
-        msg = _validate_ip_or_hostname(host)
+        # This must be an IP address only
+        msg = _validate_ip_address(host)
         if msg:
             msg = _("'%(host)s' is not a valid nameserver. %(msg)s") % {
                 'host': host, 'msg': msg}
@@ -366,41 +352,6 @@ def _validate_subnet_or_none(data, valid_values=None):
     return _validate_subnet(data, valid_values)
 
 
-def _validate_hostname(data):
-    # NOTE: An individual name regex instead of an entire FQDN was used
-    # because its easier to make correct. Feel free to replace with a
-    # full regex solution. The logic should validate that the hostname
-    # matches RFC 1123 (section 2.1) and RFC 952.
-    hostname_pattern = "[a-zA-Z0-9-]{1,63}$"
-    try:
-        # Trailing periods are allowed to indicate that a name is fully
-        # qualified per RFC 1034 (page 7).
-        trimmed = data if data[-1] != '.' else data[:-1]
-        if len(trimmed) > 255:
-            raise TypeError(
-                _("'%s' exceeds the 255 character hostname limit") % trimmed)
-        names = trimmed.split('.')
-        for name in names:
-            if not name:
-                raise TypeError(_("Encountered an empty component."))
-            if name[-1] == '-' or name[0] == '-':
-                raise TypeError(
-                    _("Name '%s' must not start or end with a hyphen.") % name)
-            if not re.match(hostname_pattern, name):
-                raise TypeError(
-                    _("Name '%s' must be 1-63 characters long, each of "
-                      "which can only be alphanumeric or a hyphen.") % name)
-        # RFC 1123 hints that a TLD can't be all numeric. last is a TLD if
-        # it's an FQDN.
-        if len(names) > 1 and re.match("^[0-9]+$", names[-1]):
-            raise TypeError(_("TLD '%s' must not be all numeric") % names[-1])
-    except TypeError as e:
-        msg = _("'%(data)s' is not a valid hostname. Reason: %(reason)s") % {
-            'data': data, 'reason': e.message}
-        LOG.debug(msg)
-        return msg
-
-
 def _validate_regex(data, valid_values=None):
     try:
         if re.match(valid_values, data):
index daa7b0930e425402fe518c9293e9241125d36d78..7a79d900a26209fc036e02b7e545701502745451 100644 (file)
@@ -328,6 +328,12 @@ class TestAttributes(base.BaseTestCase):
         ns_pools = [['1.1.1.2', '1.1.1.2'],
                     ['www.hostname.com', 'www.hostname.com'],
                     ['1000.0.0.1'],
+                    ['www.hostname.com'],
+                    ['www.great.marathons.to.travel'],
+                    ['valid'],
+                    ['77.hostname.com'],
+                    ['1' * 59],
+                    ['www.internal.hostname.com'],
                     None]
 
         for ns in ns_pools:
@@ -335,12 +341,7 @@ class TestAttributes(base.BaseTestCase):
             self.assertIsNotNone(msg)
 
         ns_pools = [['100.0.0.2'],
-                    ['www.hostname.com'],
-                    ['www.great.marathons.to.travel'],
-                    ['valid'],
-                    ['77.hostname.com'],
-                    ['1' * 59],
-                    ['www.internal.hostname.com']]
+                    ['1.1.1.1', '1.1.1.2']]
 
         for ns in ns_pools:
             msg = attributes._validate_nameservers(ns, None)
@@ -389,21 +390,6 @@ class TestAttributes(base.BaseTestCase):
         msg = attributes._validate_ip_address_or_none(ip_addr)
         self.assertEqual("'%s' is not a valid IP address" % ip_addr, msg)
 
-    def test_hostname_pattern(self):
-        bad_values = ['@openstack', 'ffff.abcdefg' * 26, 'f' * 80, '-hello',
-                      'goodbye-', 'example..org']
-        for data in bad_values:
-            msg = attributes._validate_hostname(data)
-            self.assertIsNotNone(msg)
-
-        # All numeric hostnames are allowed per RFC 1123 section 2.1
-        good_values = ['www.openstack.org', '1234x', '1234',
-                       'openstack-1', 'v.xyz', '1' * 50, 'a1a',
-                       'x.x1x', 'x.yz', 'example.org.']
-        for data in good_values:
-            msg = attributes._validate_hostname(data)
-            self.assertIsNone(msg)
-
     def test_uuid_pattern(self):
         data = 'garbage'
         msg = attributes._validate_regex(data, attributes.UUID_PATTERN)