diff --git a/quantum/plugins/nicira/nicira_nvp_plugin/nicira_qos_db.py b/quantum/plugins/nicira/nicira_nvp_plugin/nicira_qos_db.py index c79412446e..8f1127ec4d 100644 --- a/quantum/plugins/nicira/nicira_nvp_plugin/nicira_qos_db.py +++ b/quantum/plugins/nicira/nicira_nvp_plugin/nicira_qos_db.py @@ -25,6 +25,7 @@ from quantum.db import models_v2 from quantum.openstack.common import uuidutils from quantum.plugins.nicira.nicira_nvp_plugin.extensions import (nvp_qos as ext_qos) +from quantum.plugins.nicira.nicira_nvp_plugin import nvplib class QoSQueue(model_base.BASEV2, models_v2.HasId, models_v2.HasTenant): @@ -297,4 +298,7 @@ class NVPQoSDbMixin(ext_qos.QueuePluginBase): for api_name, nvp_name in params.iteritems() if attr.is_attr_set(queue.get(api_name)) ) + if 'display_name' in nvp_queue: + nvp_queue['display_name'] = nvplib._check_and_truncate_name( + nvp_queue['display_name']) return nvp_queue diff --git a/quantum/plugins/nicira/nicira_nvp_plugin/nvplib.py b/quantum/plugins/nicira/nicira_nvp_plugin/nvplib.py index 2b81ff1495..dd2854e330 100644 --- a/quantum/plugins/nicira/nicira_nvp_plugin/nvplib.py +++ b/quantum/plugins/nicira/nicira_nvp_plugin/nvplib.py @@ -54,6 +54,8 @@ LROUTERNAT_RESOURCE = "nat/lrouter" LQUEUE_RESOURCE = "lqueue" GWSERVICE_RESOURCE = "gateway-service" QUANTUM_VERSION = "2013.1" +# Other constants for NVP resource +MAX_DISPLAY_NAME_LEN = 40 # Constants for NAT rules MATCH_KEYS = ["destination_ip_addresses", "destination_port_max", "destination_port_min", "source_ip_addresses", @@ -142,6 +144,14 @@ def _build_uri_path(resource, return uri_path +def _check_and_truncate_name(display_name): + if display_name and len(display_name) > MAX_DISPLAY_NAME_LEN: + LOG.debug(_("Specified name:'%s' exceeds maximum length. " + "It will be truncated on NVP"), display_name) + return display_name[:MAX_DISPLAY_NAME_LEN] + return display_name + + def get_cluster_version(cluster): """Return major/minor version #""" # Get control-cluster nodes @@ -286,7 +296,7 @@ def create_lswitch(cluster, tenant_id, display_name, cluster.default_tz_uuid), "transport_type": (nvp_binding_type or DEF_TRANSPORT_TYPE)} - lswitch_obj = {"display_name": display_name, + lswitch_obj = {"display_name": _check_and_truncate_name(display_name), "transport_zones": [transport_zone_config], "tags": [{"tag": tenant_id, "scope": "os_tid"}]} if nvp_binding_type == 'bridge' and vlan_id: @@ -315,9 +325,7 @@ def create_lswitch(cluster, tenant_id, display_name, def update_lswitch(cluster, lswitch_id, display_name, tenant_id=None, **kwargs): uri = _build_uri_path(LSWITCH_RESOURCE, resource_id=lswitch_id) - # TODO(salvatore-orlando): Make sure this operation does not remove - # any other important tag set on the lswtich object - lswitch_obj = {"display_name": display_name, + lswitch_obj = {"display_name": _check_and_truncate_name(display_name), "tags": [{"tag": tenant_id, "scope": "os_tid"}]} if "tags" in kwargs: lswitch_obj["tags"].extend(kwargs["tags"]) @@ -354,7 +362,7 @@ def create_l2_gw_service(cluster, tenant_id, display_name, devices): "device_id": device['interface_name'], "type": "L2Gateway"} for device in devices] gwservice_obj = { - "display_name": display_name, + "display_name": _check_and_truncate_name(display_name), "tags": tags, "gateways": gateways, "type": "L2GatewayServiceConfig" @@ -382,6 +390,7 @@ def create_lrouter(cluster, tenant_id, display_name, nexthop): with the NVP controller """ tags = [{"tag": tenant_id, "scope": "os_tid"}] + display_name = _check_and_truncate_name(display_name) lrouter_obj = { "display_name": display_name, "tags": tags, @@ -493,7 +502,7 @@ def update_l2_gw_service(cluster, gateway_id, display_name): if not display_name: # Nothing to update return gwservice_obj - gwservice_obj["display_name"] = display_name + gwservice_obj["display_name"] = _check_and_truncate_name(display_name) try: return json.loads(do_single_request("PUT", _build_uri_path(GWSERVICE_RESOURCE, @@ -513,7 +522,8 @@ def update_lrouter(cluster, lrouter_id, display_name, nexthop): # Nothing to update return lrouter_obj # It seems that this is faster than the doing an if on display_name - lrouter_obj["display_name"] = display_name or lrouter_obj["display_name"] + lrouter_obj["display_name"] = (_check_and_truncate_name(display_name) or + lrouter_obj["display_name"]) if nexthop: nh_element = lrouter_obj["routing_config"].get( "default_route_next_hop") @@ -705,16 +715,14 @@ def update_port(cluster, lswitch_uuid, lport_uuid, quantum_port_id, tenant_id, display_name, device_id, admin_status_enabled, mac_address=None, fixed_ips=None, port_security_enabled=None, security_profiles=None, queue_id=None): - # device_id can be longer than 40 so we rehash it hashed_device_id = hashlib.sha1(device_id).hexdigest() lport_obj = dict( admin_status_enabled=admin_status_enabled, - display_name=display_name, + display_name=_check_and_truncate_name(display_name), tags=[dict(scope='os_tid', tag=tenant_id), dict(scope='q_port_id', tag=quantum_port_id), dict(scope='vm_id', tag=hashed_device_id)]) - _configure_extensions(lport_obj, mac_address, fixed_ips, port_security_enabled, security_profiles, queue_id) @@ -741,6 +749,7 @@ def create_lport(cluster, lswitch_uuid, tenant_id, quantum_port_id, """ Creates a logical port on the assigned logical switch """ # device_id can be longer than 40 so we rehash it hashed_device_id = hashlib.sha1(device_id).hexdigest() + display_name = _check_and_truncate_name(display_name) lport_obj = dict( admin_status_enabled=admin_status_enabled, display_name=display_name, @@ -1048,8 +1057,9 @@ def create_security_profile(cluster, tenant_id, security_profile): 'ip_prefix': '0.0.0.0/0'}], 'logical_port_ingress_rules': []} try: + display_name = _check_and_truncate_name(security_profile.get('name')) body = mk_body( - tags=tags, display_name=security_profile.get('name'), + tags=tags, display_name=display_name, logical_port_ingress_rules=dhcp['logical_port_ingress_rules'], logical_port_egress_rules=dhcp['logical_port_egress_rules']) rsp = do_request(HTTP_POST, path, body, cluster=cluster) diff --git a/quantum/tests/unit/nicira/fake_nvpapiclient.py b/quantum/tests/unit/nicira/fake_nvpapiclient.py index 772343c566..0ddae75853 100644 --- a/quantum/tests/unit/nicira/fake_nvpapiclient.py +++ b/quantum/tests/unit/nicira/fake_nvpapiclient.py @@ -23,6 +23,17 @@ from quantum.plugins.nicira.nicira_nvp_plugin import NvpApiClient LOG = logging.getLogger(__name__) +MAX_NAME_LEN = 40 + + +def _validate_name(name): + if name and len(name) > MAX_NAME_LEN: + raise Exception("Logical switch name exceeds %d characters", + MAX_NAME_LEN) + + +def _validate_resource(body): + _validate_name(body.get('display_name')) class FakeClient: @@ -104,6 +115,15 @@ class FakeClient: _fake_lqueue_dict = {} _fake_gatewayservice_dict = {} + _validators = { + LSWITCH_RESOURCE: _validate_resource, + LSWITCH_LPORT_RESOURCE: _validate_resource, + LROUTER_LPORT_RESOURCE: _validate_resource, + SECPROF_RESOURCE: _validate_resource, + LQUEUE_RESOURCE: _validate_resource, + GWSERVICE_RESOURCE: _validate_resource + } + def __init__(self, fake_files_path): self.fake_files_path = fake_files_path @@ -436,6 +456,10 @@ class FakeClient: with open("%s/%s" % (self.fake_files_path, response_file)) as f: response_template = f.read() add_resource = getattr(self, '_add_%s' % res_type) + body_json = json.loads(body) + val_func = self._validators.get(res_type) + if val_func: + val_func(body_json) args = [body] if len(uuids): args.append(uuids[0]) @@ -456,9 +480,13 @@ class FakeClient: is_attachment = True res_type = res_type[:res_type.index('attachment')] res_dict = getattr(self, '_fake_%s_dict' % res_type) + body_json = json.loads(body) + val_func = self._validators.get(res_type) + if val_func: + val_func(body_json) resource = res_dict[uuids[-1]] if not is_attachment: - resource.update(json.loads(body)) + resource.update(body_json) else: relations = resource.get("_relations", {}) body_2 = json.loads(body) diff --git a/quantum/tests/unit/nicira/test_nicira_plugin.py b/quantum/tests/unit/nicira/test_nicira_plugin.py index a5791a39f8..e003bf27b4 100644 --- a/quantum/tests/unit/nicira/test_nicira_plugin.py +++ b/quantum/tests/unit/nicira/test_nicira_plugin.py @@ -150,6 +150,12 @@ class TestNiciraPortsV2(test_plugin.TestPortsV2, NiciraPluginV2TestCase): self.assertEqual(res['port']['fixed_ips'], data['port']['fixed_ips']) + def test_create_port_name_exceeds_40_chars(self): + name = 'this_is_a_port_whose_name_is_longer_than_40_chars' + with self.port(name=name) as port: + # Assert the Quantum name is not truncated + self.assertEqual(name, port['port']['name']) + class TestNiciraNetworksV2(test_plugin.TestNetworksV2, NiciraPluginV2TestCase): @@ -228,6 +234,12 @@ class TestNiciraNetworksV2(test_plugin.TestNetworksV2, # tenant must see a single network self.assertEqual(len(res['networks']), 1) + def test_create_network_name_exceeds_40_chars(self): + name = 'this_is_a_network_whose_name_is_longer_than_40_chars' + with self.network(name=name) as net: + # Assert Quantum name is not truncated + self.assertEqual(net['network']['name'], name) + class NiciraPortSecurityTestCase(psec.PortSecurityDBTestCase): @@ -285,7 +297,12 @@ class NiciraSecurityGroupsTestCase(ext_sg.SecurityGroupDBTestCase): class TestNiciraSecurityGroup(ext_sg.TestSecurityGroups, NiciraSecurityGroupsTestCase): - pass + + def test_create_security_group_name_exceeds_40_chars(self): + name = 'this_is_a_secgroup_whose_name_is_longer_than_40_chars' + with self.security_group(name=name) as sg: + # Assert Quantum name is not truncated + self.assertEqual(sg['security_group']['name'], name) class TestNiciraL3NatTestCase(test_l3_plugin.L3NatDBTestCase, @@ -411,6 +428,12 @@ class TestNiciraL3NatTestCase(test_l3_plugin.L3NatDBTestCase, def _nvp_metadata_teardown(self): cfg.CONF.set_override('enable_metadata_access_network', False, 'NVP') + def test_create_router_name_exceeds_40_chars(self): + name = 'this_is_a_router_whose_name_is_longer_than_40_chars' + with self.router(name=name) as rtr: + # Assert Quantum name is not truncated + self.assertEqual(rtr['router']['name'], name) + def test_router_add_interface_subnet_with_metadata_access(self): self._nvp_metadata_setup() self.test_router_add_interface_subnet() @@ -571,6 +594,12 @@ class TestNiciraQoSQueue(NiciraPluginV2TestCase): self.assertEqual(q['qos_queue']['qos_marking'], 'untrusted') self.assertFalse(q['qos_queue']['default']) + def test_create_qos_queue_name_exceeds_40_chars(self): + name = 'this_is_a_queue_whose_name_is_longer_than_40_chars' + with self.qos_queue(name=name) as queue: + # Assert Quantum name is not truncated + self.assertEqual(queue['qos_queue']['name'], name) + def test_create_qos_queue_default(self): with self.qos_queue(default=True) as q: self.assertTrue(q['qos_queue']['default']) @@ -842,6 +871,12 @@ class TestNiciraNetworkGateway(test_l2_gw.NetworkGatewayDbTestCase, cfg.CONF.set_override('api_extensions_path', ext_path) super(TestNiciraNetworkGateway, self).setUp() + def test_create_network_gateway_name_exceeds_40_chars(self): + name = 'this_is_a_gateway_whose_name_is_longer_than_40_chars' + with self._network_gateway(name=name) as nw_gw: + # Assert Quantum name is not truncated + self.assertEqual(nw_gw[self.resource]['name'], name) + def test_list_network_gateways(self): with self._network_gateway(name='test-gw-1') as gw1: with self._network_gateway(name='test_gw_2') as gw2: