From 8ecb28dd09aedc0df6b91af4712aa8469a90e850 Mon Sep 17 00:00:00 2001 From: Martin Hickey Date: Thu, 3 Mar 2016 12:20:27 +0000 Subject: [PATCH] Integrate the port allowed address pairs VersionedObject in Neutron This patch is dependent on commit I8d03528f8f45f5f50fa467b39245a513a37c5d89. It integrates the VersionedObject with the existing code. Integration revealed that using IPAddress is not correct for allowed address pairs, because the address can also represent a subnet. Another issue revealed by the integration is that we must retain the original string format passed by users through API for MAC addresses. Neither we can use IPNetworkField from oslo.versionedobjects for ip_address field because it will then always append prefix length to base network address, even if prefix length is maximum for the type of IP network (meaning, the address actually represents a single host), which is contradictory to how API currently behaves (returning mask-less addresses for /32 - for ipv4 - and /128 - for ipv6 - prefix lengths). To solve those issues, 'authentic' flavors for netaddr.EUI and netaddr.IPNetwork types are introduced. Those 'authentic' flavors attempt to retain the original string representation, as passed by the caller. Since base IPNetworkField recreates network object on coerce(), and hence looses information about the original string representation, we introduce our custom flavor of the field type that reuses the network object passed by the caller. The change for the type of ip_address field triggers hash change. Anyway, we are safe to change it without considering backwards compatibility, because the object is not used anywhere yet. Co-Authored-By: Ihar Hrachyshka Change-Id: I3c937267ce789ed510373616713b3fa9517c18ac Partial-Bug: #1541928 --- neutron/common/utils.py | 36 +++++++++++++ neutron/db/allowed_address_pairs/__init__.py | 0 neutron/db/allowed_address_pairs/models.py | 30 +++++++++++ neutron/db/allowedaddresspairs_db.py | 52 +++++++++---------- neutron/db/migration/models/head.py | 2 +- neutron/db/securitygroups_rpc_base.py | 2 +- neutron/objects/common_types.py | 18 +++++++ .../port/extensions/allowedaddresspairs.py | 15 +++--- neutron/tests/unit/common/test_utils.py | 24 +++++++++ neutron/tests/unit/objects/test_base.py | 1 + .../tests/unit/objects/test_common_types.py | 23 ++++++++ neutron/tests/unit/objects/test_objects.py | 2 +- 12 files changed, 169 insertions(+), 36 deletions(-) create mode 100644 neutron/db/allowed_address_pairs/__init__.py create mode 100644 neutron/db/allowed_address_pairs/models.py diff --git a/neutron/common/utils.py b/neutron/common/utils.py index ab49087f7c0..9744c6a3c15 100644 --- a/neutron/common/utils.py +++ b/neutron/common/utils.py @@ -647,3 +647,39 @@ def transaction_guard(f): "transaction.")) return f(self, context, *args, **kwargs) return inner + + +class _AuthenticBase(object): + def __init__(self, addr, **kwargs): + super(_AuthenticBase, self).__init__(addr, **kwargs) + self._initial_value = addr + + def __str__(self): + if isinstance(self._initial_value, six.string_types): + return self._initial_value + return super(_AuthenticBase, self).__str__() + + # NOTE(ihrachys): override deepcopy because netaddr.* classes are + # slot-based and hence would not copy _initial_value + def __deepcopy__(self, memo): + return self.__class__(self._initial_value) + + +class AuthenticEUI(_AuthenticBase, netaddr.EUI): + ''' + This class retains the format of the MAC address string passed during + initialization. + + This is useful when we want to make sure that we retain the format passed + by a user through API. + ''' + + +class AuthenticIPNetwork(_AuthenticBase, netaddr.IPNetwork): + ''' + This class retains the format of the IP network string passed during + initialization. + + This is useful when we want to make sure that we retain the format passed + by a user through API. + ''' diff --git a/neutron/db/allowed_address_pairs/__init__.py b/neutron/db/allowed_address_pairs/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/neutron/db/allowed_address_pairs/models.py b/neutron/db/allowed_address_pairs/models.py new file mode 100644 index 00000000000..b46ef9fd274 --- /dev/null +++ b/neutron/db/allowed_address_pairs/models.py @@ -0,0 +1,30 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import sqlalchemy as sa +from sqlalchemy import orm + +from neutron.db import model_base +from neutron.db import models_v2 + + +class AllowedAddressPair(model_base.BASEV2): + port_id = sa.Column(sa.String(36), + sa.ForeignKey('ports.id', ondelete="CASCADE"), + primary_key=True) + mac_address = sa.Column(sa.String(32), nullable=False, primary_key=True) + ip_address = sa.Column(sa.String(64), nullable=False, primary_key=True) + + port = orm.relationship( + models_v2.Port, + backref=orm.backref("allowed_address_pairs", + lazy="joined", cascade="delete")) diff --git a/neutron/db/allowedaddresspairs_db.py b/neutron/db/allowedaddresspairs_db.py index b44ceaef086..f1a5fe93a91 100644 --- a/neutron/db/allowedaddresspairs_db.py +++ b/neutron/db/allowedaddresspairs_db.py @@ -14,28 +14,15 @@ # from neutron_lib.api import validators -from oslo_db import exception as db_exc -import sqlalchemy as sa -from sqlalchemy import orm from neutron.api.v2 import attributes as attr from neutron.db import db_base_plugin_v2 -from neutron.db import model_base -from neutron.db import models_v2 + +from neutron.common import utils from neutron.extensions import allowedaddresspairs as addr_pair - - -class AllowedAddressPair(model_base.BASEV2): - port_id = sa.Column(sa.String(36), - sa.ForeignKey('ports.id', ondelete="CASCADE"), - primary_key=True) - mac_address = sa.Column(sa.String(32), nullable=False, primary_key=True) - ip_address = sa.Column(sa.String(64), nullable=False, primary_key=True) - - port = orm.relationship( - models_v2.Port, - backref=orm.backref("allowed_address_pairs", - lazy="joined", cascade="delete")) +from neutron.objects import base as obj_base +from neutron.objects.port.extensions import (allowedaddresspairs + as obj_addr_pair) class AllowedAddressPairsMixin(object): @@ -51,12 +38,18 @@ class AllowedAddressPairsMixin(object): # use port.mac_address if no mac address in address pair if 'mac_address' not in address_pair: address_pair['mac_address'] = port['mac_address'] - db_pair = AllowedAddressPair( + # retain string format as passed through API + mac_address = utils.AuthenticEUI( + address_pair['mac_address']) + ip_address = utils.AuthenticIPNetwork( + address_pair['ip_address']) + pair_obj = obj_addr_pair.AllowedAddressPair( + context, port_id=port['id'], - mac_address=address_pair['mac_address'], - ip_address=address_pair['ip_address']) - context.session.add(db_pair) - except db_exc.DBDuplicateEntry: + mac_address=mac_address, + ip_address=ip_address) + pair_obj.create() + except obj_base.NeutronDbObjectDuplicateEntry: raise addr_pair.DuplicateAddressPairInRequest( mac_address=address_pair['mac_address'], ip_address=address_pair['ip_address']) @@ -64,11 +57,15 @@ class AllowedAddressPairsMixin(object): return allowed_address_pairs def get_allowed_address_pairs(self, context, port_id): - pairs = (context.session.query(AllowedAddressPair). - filter_by(port_id=port_id)) + pairs = self._get_allowed_address_pairs_objs(context, port_id) return [self._make_allowed_address_pairs_dict(pair) for pair in pairs] + def _get_allowed_address_pairs_objs(self, context, port_id): + pairs = obj_addr_pair.AllowedAddressPair.get_objects( + context, port_id=port_id) + return pairs + def _extend_port_dict_allowed_address_pairs(self, port_res, port_db): # If port_db is provided, allowed address pairs will be accessed via # sqlalchemy models. As they're loaded together with ports this @@ -84,9 +81,10 @@ class AllowedAddressPairsMixin(object): attr.PORTS, ['_extend_port_dict_allowed_address_pairs']) def _delete_allowed_address_pairs(self, context, id): - query = self._model_query(context, AllowedAddressPair) + pairs = self._get_allowed_address_pairs_objs(context, port_id=id) with context.session.begin(subtransactions=True): - query.filter(AllowedAddressPair.port_id == id).delete() + for pair in pairs: + pair.delete() def _make_allowed_address_pairs_dict(self, allowed_address_pairs, fields=None): diff --git a/neutron/db/migration/models/head.py b/neutron/db/migration/models/head.py index 1626e6833e4..5a86851f117 100644 --- a/neutron/db/migration/models/head.py +++ b/neutron/db/migration/models/head.py @@ -24,7 +24,7 @@ Based on this comparison database can be healed with healing migration. from neutron.db import address_scope_db # noqa from neutron.db import agents_db # noqa from neutron.db import agentschedulers_db # noqa -from neutron.db import allowedaddresspairs_db # noqa +from neutron.db.allowed_address_pairs import models # noqa from neutron.db import dns_db # noqa from neutron.db import dvr_mac_db # noqa from neutron.db import external_net_db # noqa diff --git a/neutron/db/securitygroups_rpc_base.py b/neutron/db/securitygroups_rpc_base.py index 53e39a0270e..7b63ab7cc9d 100644 --- a/neutron/db/securitygroups_rpc_base.py +++ b/neutron/db/securitygroups_rpc_base.py @@ -22,7 +22,7 @@ from neutron._i18n import _, _LW from neutron.common import constants as n_const from neutron.common import ipv6_utils as ipv6 from neutron.common import utils -from neutron.db import allowedaddresspairs_db as addr_pair +from neutron.db.allowed_address_pairs import models as addr_pair from neutron.db import models_v2 from neutron.db import securitygroups_db as sg_db from neutron.extensions import securitygroup as ext_sg diff --git a/neutron/objects/common_types.py b/neutron/objects/common_types.py index 7a30c06bfc0..4e79223446e 100644 --- a/neutron/objects/common_types.py +++ b/neutron/objects/common_types.py @@ -141,3 +141,21 @@ class MACAddress(obj_fields.FieldType): class MACAddressField(obj_fields.AutoTypedField): AUTO_TYPE = MACAddress() + + +class IPNetwork(obj_fields.FieldType): + """IPNetwork custom field. + + This custom field is different from the one provided by + oslo.versionedobjects library: it does not reset string representation for + the field. + """ + def coerce(self, obj, attr, value): + if not isinstance(value, netaddr.IPNetwork): + msg = _("Field value %s is not a netaddr.IPNetwork") % value + raise ValueError(msg) + return super(IPNetwork, self).coerce(obj, attr, value) + + +class IPNetworkField(obj_fields.AutoTypedField): + AUTO_TYPE = IPNetwork() diff --git a/neutron/objects/port/extensions/allowedaddresspairs.py b/neutron/objects/port/extensions/allowedaddresspairs.py index 861a3ed9c49..84ad6e832a6 100644 --- a/neutron/objects/port/extensions/allowedaddresspairs.py +++ b/neutron/objects/port/extensions/allowedaddresspairs.py @@ -10,12 +10,11 @@ # License for the specific language governing permissions and limitations # under the License. -import netaddr - from oslo_versionedobjects import base as obj_base from oslo_versionedobjects import fields as obj_fields -from neutron.db import allowedaddresspairs_db as models +from neutron.common import utils +from neutron.db.allowed_address_pairs import models from neutron.objects import base from neutron.objects import common_types @@ -32,7 +31,7 @@ class AllowedAddressPair(base.NeutronDbObject): fields = { 'port_id': obj_fields.UUIDField(), 'mac_address': common_types.MACAddressField(), - 'ip_address': obj_fields.IPAddressField(), + 'ip_address': common_types.IPNetworkField(), } # TODO(mhickey): get rid of it once we switch the db model to using @@ -51,7 +50,11 @@ class AllowedAddressPair(base.NeutronDbObject): def modify_fields_from_db(cls, db_obj): fields = super(AllowedAddressPair, cls).modify_fields_from_db(db_obj) if 'ip_address' in fields: - fields['ip_address'] = netaddr.IPAddress(fields['ip_address']) + # retain string format as stored in the database + fields['ip_address'] = utils.AuthenticIPNetwork( + fields['ip_address']) if 'mac_address' in fields: - fields['mac_address'] = netaddr.EUI(fields['mac_address']) + # retain string format as stored in the database + fields['mac_address'] = utils.AuthenticEUI( + fields['mac_address']) return fields diff --git a/neutron/tests/unit/common/test_utils.py b/neutron/tests/unit/common/test_utils.py index de724865198..33e1d0368c7 100644 --- a/neutron/tests/unit/common/test_utils.py +++ b/neutron/tests/unit/common/test_utils.py @@ -784,3 +784,27 @@ class TestPortRuleMasking(base.BaseTestCase): port_max = 5 with testtools.ExpectedException(ValueError): utils.port_rule_masking(port_min, port_max) + + +class TestAuthenticEUI(base.BaseTestCase): + + def test_retains_original_format(self): + for mac_str in ('FA-16-3E-73-A2-E9', 'fa:16:3e:73:a2:e9'): + self.assertEqual(mac_str, str(utils.AuthenticEUI(mac_str))) + + def test_invalid_values(self): + for mac in ('XXXX', 'ypp', 'g3:vvv'): + with testtools.ExpectedException(netaddr.core.AddrFormatError): + utils.AuthenticEUI(mac) + + +class TestAuthenticIPNetwork(base.BaseTestCase): + + def test_retains_original_format(self): + for addr_str in ('10.0.0.0/24', '10.0.0.10/32', '100.0.0.1'): + self.assertEqual(addr_str, str(utils.AuthenticIPNetwork(addr_str))) + + def test_invalid_values(self): + for addr in ('XXXX', 'ypp', 'g3:vvv'): + with testtools.ExpectedException(netaddr.core.AddrFormatError): + utils.AuthenticIPNetwork(addr) diff --git a/neutron/tests/unit/objects/test_base.py b/neutron/tests/unit/objects/test_base.py index faa1058df59..d50c31179c8 100644 --- a/neutron/tests/unit/objects/test_base.py +++ b/neutron/tests/unit/objects/test_base.py @@ -236,6 +236,7 @@ FIELD_TYPE_VALUE_GENERATOR_MAP = { obj_fields.ListOfObjectsField: lambda: [], common_types.DscpMarkField: get_random_dscp_mark, obj_fields.IPNetworkField: tools.get_random_ip_network, + common_types.IPNetworkField: tools.get_random_ip_network, common_types.IPNetworkPrefixLenField: tools.get_random_prefixlen, common_types.ListOfIPNetworksField: get_list_of_random_networks, common_types.IPVersionEnumField: tools.get_random_ip_version, diff --git a/neutron/tests/unit/objects/test_common_types.py b/neutron/tests/unit/objects/test_common_types.py index fb1fe096c4c..2d074228aca 100644 --- a/neutron/tests/unit/objects/test_common_types.py +++ b/neutron/tests/unit/objects/test_common_types.py @@ -115,6 +115,29 @@ class MACAddressFieldTest(test_base.BaseTestCase, TestField): self.assertEqual('%s' % in_val, self.field.stringify(in_val)) +class IPNetworkFieldTest(test_base.BaseTestCase, TestField): + def setUp(self): + super(IPNetworkFieldTest, self).setUp() + self.field = common_types.IPNetworkField() + addrs = [ + tools.get_random_ip_network(version=ip_version) + for ip_version in constants.IP_ALLOWED_VERSIONS + ] + self.coerce_good_values = [(addr, addr) for addr in addrs] + self.coerce_bad_values = [ + 'ypp', 'g3:vvv', + # the field type is strict and does not allow to pass strings, even + # if they represent a valid IP network + '10.0.0.0/24', + ] + self.to_primitive_values = self.coerce_good_values + self.from_primitive_values = self.coerce_good_values + + def test_stringify(self): + for in_val, out_val in self.coerce_good_values: + self.assertEqual('%s' % in_val, self.field.stringify(in_val)) + + class IPVersionEnumFieldTest(test_base.BaseTestCase, TestField): def setUp(self): super(IPVersionEnumFieldTest, self).setUp() diff --git a/neutron/tests/unit/objects/test_objects.py b/neutron/tests/unit/objects/test_objects.py index a8c356eae9d..312f6698614 100644 --- a/neutron/tests/unit/objects/test_objects.py +++ b/neutron/tests/unit/objects/test_objects.py @@ -29,7 +29,7 @@ object_data = { 'AddressScope': '1.0-681cb915f973c92350fe2c797dec2ea4', 'ExtraDhcpOpt': '1.0-632f689cbeb36328995a7aed1d0a78d3', 'PortSecurity': '1.0-cf5b382a0112080ec4e0f23f697c7ab2', - 'AllowedAddressPair': '1.0-0d7380d7d4a32f72e6ae509af1476297', + 'AllowedAddressPair': '1.0-9f9186b6f952fbf31d257b0458b852c0', 'QosBandwidthLimitRule': '1.1-4e44a8f5c2895ab1278399f87b40a13d', 'QosDscpMarkingRule': '1.1-0313c6554b34fd10c753cb63d638256c', 'QosRuleType': '1.1-8a53fef4c6a43839d477a85b787d22ce',