From 36ce472ace244c2c5124423c6f4e23f53360b9b3 Mon Sep 17 00:00:00 2001 From: ghanshyam Date: Mon, 13 Jun 2016 18:55:20 +0900 Subject: [PATCH] Remove python code validation specific to legacy_v2 As legacy v2 code is removed and v2.1(v2 compatible mode) validate API request body through json schema, python code validation for some request param is not needed now. - UUID checks for sch_hint: 'group' - Done by JSON schema - create network param checking - moved to nova-manage(for API, JSON schema does validation) Adding more UT and also removing legacy_v2 files from py34 blacklist tests file. Partially implements blueprint remove-legacy-v2-api-code Change-Id: I2e247051c982e894b81c1ef83aa12dc9d56d94da --- nova/cmd/manage.py | 10 ++++ nova/compute/api.py | 6 -- nova/hacking/checks.py | 3 +- nova/network/manager.py | 18 ------ .../api/openstack/compute/test_networks.py | 14 ++++- nova/tests/unit/compute/test_compute.py | 12 ---- nova/tests/unit/test_hacking.py | 4 -- nova/tests/unit/test_nova_manage.py | 57 +++++++++++++++++++ tests-py3.txt | 7 --- 9 files changed, 81 insertions(+), 50 deletions(-) diff --git a/nova/cmd/manage.py b/nova/cmd/manage.py index 21a697f012de..f86954286676 100644 --- a/nova/cmd/manage.py +++ b/nova/cmd/manage.py @@ -540,6 +540,16 @@ class NetworkCommands(object): dns1=None, dns2=None, project_id=None, priority=None, uuid=None, fixed_cidr=None): """Creates fixed IPs for host by range.""" + + # NOTE(gmann): These checks are moved here as API layer does all these + # validation through JSON schema. + if not label: + raise exception.NetworkNotCreated(req="label") + if len(label) > 255: + raise exception.LabelTooLong() + if not (cidr or cidr_v6): + raise exception.NetworkNotCreated(req="cidr or cidr_v6") + kwargs = {k: v for k, v in six.iteritems(locals()) if v and k != "self"} if multi_host is not None: diff --git a/nova/compute/api.py b/nova/compute/api.py index ad4765cc573a..2a23aaac78ab 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -1050,12 +1050,6 @@ class API(base.Base): if not group_hint: return - # TODO(gibi): We need to remove the following validation code when - # removing legacy v2 code. - if not uuidutils.is_uuid_like(group_hint): - msg = _('Server group scheduler hint must be a UUID.') - raise exception.InvalidInput(reason=msg) - return objects.InstanceGroup.get_by_uuid(context, group_hint) def _create_instance(self, context, instance_type, diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py index 4506c2427959..43c0388bca5e 100644 --- a/nova/hacking/checks.py +++ b/nova/hacking/checks.py @@ -594,8 +594,7 @@ def check_http_not_implemented(logical_line, physical_line, filename): " common raise_feature_not_supported().") if pep8.noqa(physical_line): return - if ("nova/api/openstack/compute/legacy_v2" in filename or - "nova/api/openstack/compute" not in filename): + if ("nova/api/openstack/compute" not in filename): return if re.match(http_not_implemented_re, logical_line): yield(0, msg) diff --git a/nova/network/manager.py b/nova/network/manager.py index 2d53b6b63918..1db3cc65a0c8 100644 --- a/nova/network/manager.py +++ b/nova/network/manager.py @@ -1105,24 +1105,6 @@ class NetworkManager(manager.Manager): kwargs[name] = locals()[name] self._convert_int_args(kwargs) - # check for certain required inputs - # NOTE: We can remove this check after v2.0 API code is removed because - # jsonschema has checked already before this. - label = kwargs["label"] - if not label: - raise exception.NetworkNotCreated(req="label") - - # Size of "label" column in nova.networks is 255, hence the restriction - # NOTE: We can remove this check after v2.0 API code is removed because - # jsonschema has checked already before this. - if len(label) > 255: - raise exception.LabelTooLong() - - # NOTE: We can remove this check after v2.0 API code is removed because - # jsonschema has checked already before this. - if not (kwargs["cidr"] or kwargs["cidr_v6"]): - raise exception.NetworkNotCreated(req="cidr or cidr_v6") - kwargs["bridge"] = kwargs["bridge"] or CONF.flat_network_bridge kwargs["bridge_interface"] = (kwargs["bridge_interface"] or CONF.flat_interface) diff --git a/nova/tests/unit/api/openstack/compute/test_networks.py b/nova/tests/unit/api/openstack/compute/test_networks.py index ec7aadbc2183..7205844002ef 100644 --- a/nova/tests/unit/api/openstack/compute/test_networks.py +++ b/nova/tests/unit/api/openstack/compute/test_networks.py @@ -257,7 +257,19 @@ class NetworkCreateExceptionsTestV21(test.TestCase): body=self.new_network) def test_network_create_no_cidr(self): - self.new_network['network']['cidr'] = '' + del self.new_network['network']['cidr'] + self.assertRaises(self.validation_error, + self.controller.create, self.req, + body=self.new_network) + + def test_network_create_no_label(self): + del self.new_network['network']['label'] + self.assertRaises(self.validation_error, + self.controller.create, self.req, + body=self.new_network) + + def test_network_create_label_too_long(self): + self.new_network['network']['label'] = "x" * 256 self.assertRaises(self.validation_error, self.controller.create, self.req, body=self.new_network) diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 60147a9e5977..270eb9c366fa 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -7776,18 +7776,6 @@ class ComputeAPITestCase(BaseTestCase): group = objects.InstanceGroup.get_by_uuid(self.context, group.uuid) self.assertIn(refs[0]['uuid'], group.members) - def test_instance_create_with_group_name_fails(self): - self.stubs.Set(fake_image._FakeImageService, 'show', self.fake_show) - - inst_type = flavors.get_default_flavor() - self.assertRaises( - exception.InvalidInput, - self.compute_api.create, - self.context, - inst_type, - self.fake_image['id'], - scheduler_hints={'group': 'non-uuid'}) - def test_instance_create_with_group_uuid_fails_group_not_exist(self): self.stub_out('nova.tests.unit.image.fake._FakeImageService.show', self.fake_show) diff --git a/nova/tests/unit/test_hacking.py b/nova/tests/unit/test_hacking.py index 0f80d568f09e..0c85a1060b8b 100644 --- a/nova/tests/unit/test_hacking.py +++ b/nova/tests/unit/test_hacking.py @@ -531,10 +531,6 @@ class HackingTestCase(test.NoDBTestCase): self._assert_has_errors(code, checks.check_http_not_implemented, expected_errors=errors, filename=filename) - filename = "nova/api/openstack/compute/legacy_v2/test.py" - self._assert_has_no_errors(code, checks.check_http_not_implemented, - filename=filename) - def test_check_contextlib_use(self): code = """ with test.nested( diff --git a/nova/tests/unit/test_nova_manage.py b/nova/tests/unit/test_nova_manage.py index a1a6a2581d8d..9045ba1ded5d 100644 --- a/nova/tests/unit/test_nova_manage.py +++ b/nova/tests/unit/test_nova_manage.py @@ -210,6 +210,63 @@ class NetworkCommandsTestCase(test.NoDBTestCase): dns2='8.8.4.4', uuid='aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa') + def test_create_without_lable(self): + self.assertRaises(exception.NetworkNotCreated, + self.commands.create, + cidr='10.2.0.0/24', + num_networks=1, + network_size=256, + multi_host='F', + vlan=200, + vlan_start=201, + vpn_start=2000, + cidr_v6='fd00:2::/120', + gateway='10.2.0.1', + gateway_v6='fd00:2::22', + bridge='br200', + bridge_interface='eth0', + dns1='8.8.8.8', + dns2='8.8.4.4', + uuid='aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa') + + def test_create_with_lable_too_long(self): + self.assertRaises(exception.LabelTooLong, + self.commands.create, + label='x' * 256, + cidr='10.2.0.0/24', + num_networks=1, + network_size=256, + multi_host='F', + vlan=200, + vlan_start=201, + vpn_start=2000, + cidr_v6='fd00:2::/120', + gateway='10.2.0.1', + gateway_v6='fd00:2::22', + bridge='br200', + bridge_interface='eth0', + dns1='8.8.8.8', + dns2='8.8.4.4', + uuid='aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa') + + def test_create_without_cidr(self): + self.assertRaises(exception.NetworkNotCreated, + self.commands.create, + label='Test', + num_networks=1, + network_size=256, + multi_host='F', + vlan=200, + vlan_start=201, + vpn_start=2000, + gateway='10.2.0.1', + gateway_v6='fd00:2::22', + bridge='br200', + bridge_interface='eth0', + dns1='8.8.8.8', + dns2='8.8.4.4', + uuid='aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa') + def test_list(self): def fake_network_get_all(context): diff --git a/tests-py3.txt b/tests-py3.txt index b309d8254559..2648497f29e1 100644 --- a/tests-py3.txt +++ b/tests-py3.txt @@ -1,10 +1,3 @@ -nova.tests.unit.api.openstack.compute.legacy_v2.test_extensions.ActionExtensionTest -nova.tests.unit.api.openstack.compute.legacy_v2.test_extensions.ControllerExtensionTest -nova.tests.unit.api.openstack.compute.legacy_v2.test_extensions.ExtensionControllerIdFormatTest -nova.tests.unit.api.openstack.compute.legacy_v2.test_extensions.ExtensionManagerTest -nova.tests.unit.api.openstack.compute.legacy_v2.test_extensions.ResourceExtensionTest -nova.tests.unit.api.openstack.compute.legacy_v2.test_servers.ServersControllerCreateTest -nova.tests.unit.api.openstack.compute.legacy_v2.test_servers.ServersControllerTest nova.tests.unit.api.openstack.compute.test_api.APITest nova.tests.unit.api.openstack.compute.test_api.APITestV21 nova.tests.unit.api.openstack.compute.test_console_output.ConsoleOutputExtensionTestV2