Enhancements to CIDR and IP address constraints.

Currently the constraints do not reject an ipaddress for ipv4 which have
fewer than 3 dots such as 'a' or 'a.b' or 'a.b.c'.
This enhancement provides an extra check that an ipv4 address has syntax:
'a.b.c.d'

This also applies to CIDR

Change-Id: Ia7ec8bf107abd169b6b6a91d0b8bb913fc3cc7b9
Story: 2002552
Task: 22114

heat/engine/constraint/common_constraints.py

@@ -17,7 +17,7 @@ import netaddr
 import pytz
 import six
 
-from oslo_utils import netutils
+from neutron_lib.api import validators
 from oslo_utils import timeutils
 
 from heat.common.i18n import _
@@ -35,7 +35,12 @@ class IPConstraint(constraints.BaseCustomConstraint):
 
     def validate(self, value, context, template=None):
         self._error_message = 'Invalid IP address'
-        return netutils.is_valid_ip(value)
+        if not isinstance(value, six.string_types):
+            return False
+        msg = validators.validate_ip_address(value)
+        if msg is not None:
+            return False
+        return True
 
 
 class MACConstraint(constraints.BaseCustomConstraint):
@@ -100,17 +105,14 @@ class DNSDomainConstraint(DNSNameConstraint):
 
 class CIDRConstraint(constraints.BaseCustomConstraint):
 
-    def _validate_whitespace(self, data):
-        self._error_message = ("Invalid net cidr '%s' contains "
-                               "whitespace" % data)
-        if len(data.split()) > 1:
-            return False
-        return True
-
     def validate(self, value, context, template=None):
         try:
-            netaddr.IPNetwork(value)
-            return self._validate_whitespace(value)
+            netaddr.IPNetwork(value, implicit_prefix=True)
+            msg = validators.validate_subnet(value)
+            if msg is not None:
+                self._error_message = msg
+                return False
+            return True
         except Exception as ex:
             self._error_message = 'Invalid net cidr %s ' % six.text_type(ex)
             return False

heat/tests/constraints/test_common_constraints.py

@@ -11,6 +11,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
 import six
 
 from heat.engine.constraint import common_constraints as cc
@@ -35,6 +36,11 @@ class TestIPConstraint(common.HeatTestCase):
 
     def test_invalidate_ipv4_format(self):
         invalidate_format = [
+            None,
+            123,
+            '1.1',
+            '1.1.',
+            '1.1.1',
             '1.1.1.',
             '1.1.1.256',
             'invalidate format',
@@ -100,7 +106,6 @@ class TestCIDRConstraint(common.HeatTestCase):
         validate_format = [
             '10.0.0.0/24',
             '6000::/64',
-            '8.8.8.8'
         ]
         for cidr in validate_format:
             self.assertTrue(self.constraint.validate(cidr, None))
@@ -111,11 +116,30 @@ class TestCIDRConstraint(common.HeatTestCase):
             'Invalid cidr',
             '300.0.0.0/24',
             '10.0.0.0/33',
-            '8.8.8.0/ 24'
+            '10.0.0/24',
+            '10.0/24',
+            '10.0.a.10/24',
+            '8.8.8.0/ 24',
+            '8.8.8.8'
         ]
         for cidr in invalidate_format:
             self.assertFalse(self.constraint.validate(cidr, None))
 
+    @mock.patch('neutron_lib.api.validators.validate_subnet')
+    def test_validate(self, mock_validate_subnet):
+        test_formats = [
+            '10.0.0/24',
+            '10.0/24',
+        ]
+        self.assertFalse(self.constraint.validate('10.0.0.0/33', None))
+
+        for cidr in test_formats:
+            self.assertFalse(self.constraint.validate(cidr, None))
+
+        mock_validate_subnet.assert_any_call('10.0.0/24')
+        mock_validate_subnet.assert_called_with('10.0/24')
+        self.assertEqual(mock_validate_subnet.call_count, 2)
+
 
 class TestISO8601Constraint(common.HeatTestCase):
 

heat/tests/openstack/neutron/test_neutron_port.py

@@ -51,7 +51,7 @@ resources:
     properties:
       network: abcd1234
       allowed_address_pairs:
-        - ip_address: 10.0.3.21
+        - ip_address: 10.0.3.21/8
           mac_address: 00-B0-D0-86-BB-F7
 '''
 
@@ -198,7 +198,7 @@ class NeutronPortTest(common.HeatTestCase):
         self.create_mock.assert_called_once_with({'port': {
             'network_id': u'abcd1234',
             'allowed_address_pairs': [{
-                'ip_address': u'10.0.3.21',
+                'ip_address': u'10.0.3.21/8',
                 'mac_address': u'00-B0-D0-86-BB-F7'
             }],
             'name': utils.PhysName(stack.name, 'port'),
@@ -258,7 +258,7 @@ class NeutronPortTest(common.HeatTestCase):
         self.create_mock.assert_called_once_with({'port': {
             'network_id': u'abcd1234',
             'allowed_address_pairs': [{
-                'ip_address': u'10.0.3.21',
+                'ip_address': u'10.0.3.21/8',
             }],
             'name': utils.PhysName(stack.name, 'port'),
             'admin_state_up': True,

heat/tests/openstack/neutron/test_neutron_security_group_rule.py

@@ -68,7 +68,7 @@ class SecurityGroupRuleTest(common.HeatTestCase):
                          return_value=(True, None))
 
         tmpl = inline_templates.SECURITY_GROUP_RULE_TEMPLATE
-        tmpl += '      remote_ip_prefix: "123"'
+        tmpl += '      remote_ip_prefix: "10.0.0.0/8"'
         self._create_stack(tmpl=tmpl)
 
         self.assertRaises(exception.ResourcePropertyConflict,

lower-constraints.txt

@@ -56,6 +56,7 @@ msgpack==0.5.6
 munch==2.2.0
 netaddr==0.7.18
 netifaces==0.10.6
+neutron-lib==1.14.0
 openstacksdk==0.11.2
 os-client-config==1.29.0
 os-service-types==1.2.0

requirements.txt

@@ -11,6 +11,7 @@ keystoneauth1>=3.4.0 # Apache-2.0
 keystonemiddleware>=4.17.0 # Apache-2.0
 lxml!=3.7.0,>=3.4.1 # BSD
 netaddr>=0.7.18 # BSD
+neutron-lib>=1.14.0  # Apache-2.0
 openstacksdk>=0.11.2 # Apache-2.0
 oslo.cache>=1.26.0 # Apache-2.0
 oslo.config>=5.2.0 # Apache-2.0