From 9592bcb59114e0f8ef8ffe82edd65a4f5a9532f2 Mon Sep 17 00:00:00 2001 From: Nakul Dahiwade Date: Wed, 13 Jun 2018 23:13:41 +0000 Subject: [PATCH] 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 | 24 ++++++++-------- .../constraints/test_common_constraints.py | 28 +++++++++++++++++-- .../openstack/neutron/test_neutron_port.py | 6 ++-- .../test_neutron_security_group_rule.py | 2 +- lower-constraints.txt | 1 + requirements.txt | 1 + 6 files changed, 45 insertions(+), 17 deletions(-) diff --git a/heat/engine/constraint/common_constraints.py b/heat/engine/constraint/common_constraints.py index 3d3637587a..124f09eb92 100644 --- a/heat/engine/constraint/common_constraints.py +++ b/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 diff --git a/heat/tests/constraints/test_common_constraints.py b/heat/tests/constraints/test_common_constraints.py index eadd317b2e..665c0b9033 100644 --- a/heat/tests/constraints/test_common_constraints.py +++ b/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): diff --git a/heat/tests/openstack/neutron/test_neutron_port.py b/heat/tests/openstack/neutron/test_neutron_port.py index 4230781090..58482e68a4 100644 --- a/heat/tests/openstack/neutron/test_neutron_port.py +++ b/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, diff --git a/heat/tests/openstack/neutron/test_neutron_security_group_rule.py b/heat/tests/openstack/neutron/test_neutron_security_group_rule.py index 5e8a5e43f8..407f5e9ce8 100644 --- a/heat/tests/openstack/neutron/test_neutron_security_group_rule.py +++ b/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, diff --git a/lower-constraints.txt b/lower-constraints.txt index 2fd1f2b519..ac802be037 100644 --- a/lower-constraints.txt +++ b/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 diff --git a/requirements.txt b/requirements.txt index 858a9cfd42..26b0df43aa 100644 --- a/requirements.txt +++ b/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