From ced5e9e3d82c68037cc90442f555adcc8c2a5d76 Mon Sep 17 00:00:00 2001 From: Hongbin Lu Date: Mon, 21 Jan 2019 05:11:59 +0000 Subject: [PATCH] Add validation on network attach/detach Change-Id: I9e4987aec424bfaa66838662eccd69799351ec37 Related-Bug: #1812602 --- zun/api/controllers/v1/containers.py | 28 +++++++++++++++++++ .../api/controllers/v1/test_containers.py | 10 +++++-- zun/tests/unit/db/utils.py | 8 ++++-- 3 files changed, 40 insertions(+), 6 deletions(-) diff --git a/zun/api/controllers/v1/containers.py b/zun/api/controllers/v1/containers.py index 6e82d4746..e0fae5f4d 100644 --- a/zun/api/controllers/v1/containers.py +++ b/zun/api/controllers/v1/containers.py @@ -1207,9 +1207,23 @@ class ContainersController(base.Controller): if kwargs.get('port'): port = neutron_api.get_neutron_port(kwargs['port']) net_id = port['network_id'] + if (net_id not in container.addresses or + port['id'] not in [a['port'] + for a in container.addresses[net_id]]): + raise exception.Invalid(_( + "Port '%(port)s' is not attached to container " + "'%(container)s'.") % + {"port": kwargs.get('port'), + "container": container_ident}) else: network = neutron_api.get_neutron_network(kwargs.get('network')) net_id = network['id'] + if net_id not in container.addresses: + raise exception.Invalid(_( + "Network '%(network)s' is not attached to container " + "'%(container)s'.") % + {"network": kwargs.get('network'), + "container": container_ident}) compute_api.network_detach(context, container, net_id) pecan.response.status = 202 @@ -1229,6 +1243,20 @@ class ContainersController(base.Controller): context = pecan.request.context compute_api = pecan.request.compute_api requested_networks = utils.build_requested_networks(context, [kwargs]) + if requested_networks[0]['network'] in container.addresses: + if kwargs.get('port'): + raise exception.Invalid(_( + "Cannot attach port '%(port)s' to container " + "'%(container)s' because another port on the " + "same network is already attached to this container.") % + {"port": kwargs.get('port'), + "container": container_ident}) + else: + raise exception.Invalid(_( + "Network '%(network)s' is already connected to " + "container '%(container)s'.") % + {"network": kwargs.get('network'), + "container": container_ident}) compute_api.network_attach(context, container, requested_networks[0]) @base.Controller.api_version("1.13", "1.17") diff --git a/zun/tests/unit/api/controllers/v1/test_containers.py b/zun/tests/unit/api/controllers/v1/test_containers.py index 2a19a427b..01e98cdf4 100644 --- a/zun/tests/unit/api/controllers/v1/test_containers.py +++ b/zun/tests/unit/api/controllers/v1/test_containers.py @@ -2027,11 +2027,15 @@ class TestContainerController(api_base.FunctionalTest): @patch('zun.objects.Container.get_by_uuid') def test_network_detach_with_port(self, mock_by_uuid, mock_detach, mock_get_port): - test_container = utils.get_test_container() + port_uuid = uuidutils.generate_uuid() + network_uuid = uuidutils.generate_uuid() + test_container = utils.get_test_container(port=port_uuid, + network=network_uuid) test_container_obj = objects.Container(self.context, **test_container) mock_by_uuid.return_value = test_container_obj container_uuid = test_container.get('uuid') - mock_get_port.return_value = {'network_id': 'fake-net-id'} + mock_get_port.return_value = {'network_id': network_uuid, + 'id': port_uuid} mock_detach.return_value = None url = '/v1/containers/%s/%s?port=%s' % (container_uuid, 'network_detach', @@ -2039,7 +2043,7 @@ class TestContainerController(api_base.FunctionalTest): response = self.post(url) self.assertEqual(202, response.status_int) mock_detach.assert_called_once_with(mock.ANY, test_container_obj, - 'fake-net-id') + network_uuid) @patch('zun.objects.Container.get_by_uuid') def test_network_list(self, mock_container_get_by_uuid): diff --git a/zun/tests/unit/db/utils.py b/zun/tests/unit/db/utils.py index 30c727a10..82060be77 100644 --- a/zun/tests/unit/db/utils.py +++ b/zun/tests/unit/db/utils.py @@ -78,20 +78,22 @@ def get_test_container(**kwargs): 'labels': kwargs.get('labels', {'key1': 'val1', 'key2': 'val2'}), 'meta': kwargs.get('meta', {'key1': 'val1', 'key2': 'val2'}), 'addresses': kwargs.get('addresses', { - 'private': [ + kwargs.get('network', 'private'): [ { 'subnet_id': 'f89ae741-999e-4873-b38c-779e3deb8458', 'version': 4, 'preserve_on_delete': False, 'addr': '172.24.4.4', - 'port': '22626847-f511-42ee-ab06-8a9764ad2680' + 'port': kwargs.get('port', + '22626847-f511-42ee-ab06-8a9764ad2680') }, { 'subnet_id': '3e4e9708-d83b-46fb-8591-8143bd66206e', 'version': 6, 'preserve_on_delete': False, 'addr': '2001:db8::5', - 'port': '22626847-f511-42ee-ab06-8a9764ad2680' + 'port': kwargs.get('port', + '22626847-f511-42ee-ab06-8a9764ad2680') } ], }),