From 1204e4dab8254815cb03c560f23ae4a8798538ae Mon Sep 17 00:00:00 2001 From: ghanshyam Date: Sun, 29 Oct 2017 17:59:42 +0300 Subject: [PATCH] Merge flavor extensions controller code As nova extensions concept has already been removed, next goal is to merge all scattered code into main controller code. Currently schema, request and response extended code are present in multiple extensions files. This commit merge the flavor extensions code into main controller view. Note: test_flavor_rxtx.py is removed as those tests are now covered in test_flavors.py Partially implements: blueprint api-extensions-merge-queens Change-Id: I7ddda37ffb1afc72256624b0ddad319f133e0dd8 --- nova/api/openstack/compute/flavor_access.py | 35 ---------- nova/api/openstack/compute/flavor_rxtx.py | 54 --------------- nova/api/openstack/compute/routes.py | 6 +- nova/api/openstack/compute/views/flavors.py | 50 ++++++++++++-- .../openstack/compute/test_flavor_access.py | 37 ----------- .../openstack/compute/test_flavor_manage.py | 4 +- .../api/openstack/compute/test_flavor_rxtx.py | 66 ------------------- .../api/openstack/compute/test_flavors.py | 60 +++++++++++++++++ 8 files changed, 109 insertions(+), 203 deletions(-) delete mode 100644 nova/api/openstack/compute/flavor_rxtx.py delete mode 100644 nova/tests/unit/api/openstack/compute/test_flavor_rxtx.py diff --git a/nova/api/openstack/compute/flavor_access.py b/nova/api/openstack/compute/flavor_access.py index 7ad7ff1cc38a..ca52622c3b27 100644 --- a/nova/api/openstack/compute/flavor_access.py +++ b/nova/api/openstack/compute/flavor_access.py @@ -58,41 +58,6 @@ class FlavorAccessController(wsgi.Controller): class FlavorActionController(wsgi.Controller): """The flavor access API controller for the OpenStack API.""" - def _extend_flavor(self, flavor_rval, flavor_ref): - key = "os-flavor-access:is_public" - flavor_rval[key] = flavor_ref['is_public'] - - @wsgi.extends - def show(self, req, resp_obj, id): - context = req.environ['nova.context'] - if context.can(fa_policies.BASE_POLICY_NAME, fatal=False): - db_flavor = req.get_db_flavor(id) - - self._extend_flavor(resp_obj.obj['flavor'], db_flavor) - - @wsgi.extends - def detail(self, req, resp_obj): - context = req.environ['nova.context'] - if context.can(fa_policies.BASE_POLICY_NAME, fatal=False): - flavors = list(resp_obj.obj['flavors']) - for flavor_rval in flavors: - db_flavor = req.get_db_flavor(flavor_rval['id']) - self._extend_flavor(flavor_rval, db_flavor) - - @wsgi.extends(action='create') - def create(self, req, body, resp_obj): - context = req.environ['nova.context'] - if context.can(fa_policies.BASE_POLICY_NAME, fatal=False): - db_flavor = req.get_db_flavor(resp_obj.obj['flavor']['id']) - - self._extend_flavor(resp_obj.obj['flavor'], db_flavor) - - @wsgi.extends(action='update') - def update(self, req, id, body, resp_obj): - context = req.environ['nova.context'] - if context.can(fa_policies.BASE_POLICY_NAME, fatal=False): - db_flavor = req.get_db_flavor(resp_obj.obj['flavor']['id']) - self._extend_flavor(resp_obj.obj['flavor'], db_flavor) @extensions.expected_errors((400, 403, 404, 409)) @wsgi.action("addTenantAccess") diff --git a/nova/api/openstack/compute/flavor_rxtx.py b/nova/api/openstack/compute/flavor_rxtx.py deleted file mode 100644 index 497490f30cf9..000000000000 --- a/nova/api/openstack/compute/flavor_rxtx.py +++ /dev/null @@ -1,54 +0,0 @@ -# Copyright 2012 Nebula, Inc. -# -# 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. - -"""The Flavor Rxtx API extension.""" - -from nova.api.openstack import wsgi -from nova.policies import flavor_rxtx as fr_policies - -ALIAS = 'os-flavor-rxtx' - - -class FlavorRxtxController(wsgi.Controller): - def _extend_flavors(self, req, flavors): - for flavor in flavors: - db_flavor = req.get_db_flavor(flavor['id']) - key = 'rxtx_factor' - flavor[key] = db_flavor['rxtx_factor'] or "" - - def _show(self, req, resp_obj): - context = req.environ['nova.context'] - if not context.can(fr_policies.BASE_POLICY_NAME, fatal=False): - return - if 'flavor' in resp_obj.obj: - self._extend_flavors(req, [resp_obj.obj['flavor']]) - - @wsgi.extends - def show(self, req, resp_obj, id): - return self._show(req, resp_obj) - - @wsgi.extends(action='create') - def create(self, req, resp_obj, body): - return self._show(req, resp_obj) - - @wsgi.extends(action='update') - def update(self, req, id, body, resp_obj): - return self._show(req, resp_obj) - - @wsgi.extends - def detail(self, req, resp_obj): - context = req.environ['nova.context'] - if not context.can(fr_policies.BASE_POLICY_NAME, fatal=False): - return - self._extend_flavors(req, list(resp_obj.obj['flavors'])) diff --git a/nova/api/openstack/compute/routes.py b/nova/api/openstack/compute/routes.py index 929f825a8f19..aa780ea26c38 100644 --- a/nova/api/openstack/compute/routes.py +++ b/nova/api/openstack/compute/routes.py @@ -43,7 +43,6 @@ from nova.api.openstack.compute import extension_info from nova.api.openstack.compute import fixed_ips from nova.api.openstack.compute import flavor_access from nova.api.openstack.compute import flavor_manage -from nova.api.openstack.compute import flavor_rxtx from nova.api.openstack.compute import flavors from nova.api.openstack.compute import flavors_extraspecs from nova.api.openstack.compute import floating_ip_dns @@ -160,10 +159,7 @@ fixed_ips_controller = functools.partial(_create_controller, flavor_controller = functools.partial(_create_controller, flavors.FlavorsController, - [ - flavor_rxtx.FlavorRxtxController, - flavor_access.FlavorActionController - ], + [], [ flavor_manage.FlavorManageController, flavor_access.FlavorActionController diff --git a/nova/api/openstack/compute/views/flavors.py b/nova/api/openstack/compute/views/flavors.py index 04f10831eff8..fcdd6fcf4ca4 100644 --- a/nova/api/openstack/compute/views/flavors.py +++ b/nova/api/openstack/compute/views/flavors.py @@ -15,6 +15,8 @@ from nova.api.openstack import api_version_request from nova.api.openstack import common +from nova.policies import flavor_access as fa_policies +from nova.policies import flavor_rxtx as fr_policies FLAVOR_DESCRIPTION_MICROVERSION = '2.55' @@ -23,7 +25,11 @@ class ViewBuilder(common.ViewBuilder): _collection_name = "flavors" - def basic(self, request, flavor, include_description=False): + def basic(self, request, flavor, include_description=False, + update_is_public=None, update_rxtx_factor=None): + # update_is_public & update_rxtx_factor are placeholder param + # which are not used in this method as basic() method is used by + # index() (GET /flavors) which does not return those keys in response. flavor_dict = { "flavor": { "id": flavor["flavorid"], @@ -39,7 +45,8 @@ class ViewBuilder(common.ViewBuilder): return flavor_dict - def show(self, request, flavor, include_description=False): + def show(self, request, flavor, include_description=False, + update_is_public=None, update_rxtx_factor=None): flavor_dict = { "flavor": { "id": flavor["flavorid"], @@ -59,6 +66,26 @@ class ViewBuilder(common.ViewBuilder): if include_description: flavor_dict['flavor']['description'] = flavor.description + # TODO(gmann): 'update_is_public' & 'update_rxtx_factor' are policies + # checks. Once os-flavor-access & os-flavor-rxtx policies are + # removed, 'os-flavor-access:is_public' and 'rxtx_factor' need to be + # added in response without any check. + + # Evaluate the policies when using show method directly. + context = request.environ['nova.context'] + if update_is_public is None: + update_is_public = context.can(fa_policies.BASE_POLICY_NAME, + fatal=False) + if update_rxtx_factor is None: + update_rxtx_factor = context.can(fr_policies.BASE_POLICY_NAME, + fatal=False) + if update_is_public: + flavor_dict['flavor'].update({ + "os-flavor-access:is_public": flavor['is_public']}) + if update_rxtx_factor: + flavor_dict['flavor'].update( + {"rxtx_factor": flavor['rxtx_factor'] or ""}) + return flavor_dict def index(self, request, flavors): @@ -74,11 +101,19 @@ class ViewBuilder(common.ViewBuilder): coll_name = self._collection_name + '/detail' include_description = api_version_request.is_supported( request, FLAVOR_DESCRIPTION_MICROVERSION) + context = request.environ['nova.context'] + update_is_public = context.can(fa_policies.BASE_POLICY_NAME, + fatal=False) + update_rxtx_factor = context.can(fr_policies.BASE_POLICY_NAME, + fatal=False) return self._list_view(self.show, request, flavors, coll_name, - include_description=include_description) + include_description=include_description, + update_is_public=update_is_public, + update_rxtx_factor=update_rxtx_factor) def _list_view(self, func, request, flavors, coll_name, - include_description=False): + include_description=False, update_is_public=None, + update_rxtx_factor=None): """Provide a view for a list of flavors. :param func: Function used to format the flavor data @@ -88,10 +123,15 @@ class ViewBuilder(common.ViewBuilder): for a pagination query :param include_description: If the flavor.description should be included in the response dict. + :param update_is_public: If the flavor.is_public field should be + included in the response dict. + :param update_rxtx_factor: If the flavor.rxtx_factor field should be + included in the response dict. :returns: Flavor reply data in dictionary format """ - flavor_list = [func(request, flavor, include_description)["flavor"] + flavor_list = [func(request, flavor, include_description, + update_is_public, update_rxtx_factor)["flavor"] for flavor in flavors] flavors_links = self._get_collection_links(request, flavors, diff --git a/nova/tests/unit/api/openstack/compute/test_flavor_access.py b/nova/tests/unit/api/openstack/compute/test_flavor_access.py index 20a73ef996f2..542f5bf0249c 100644 --- a/nova/tests/unit/api/openstack/compute/test_flavor_access.py +++ b/nova/tests/unit/api/openstack/compute/test_flavor_access.py @@ -257,28 +257,6 @@ class FlavorAccessTestV21(test.NoDBTestCase): result = self.flavor_controller.index(req) self._verify_flavor_list(result['flavors'], expected['flavors']) - def test_show(self): - resp = FakeResponse() - self.flavor_action_controller.show(self.req, resp, '0') - self.assertEqual({'id': '0', 'os-flavor-access:is_public': True}, - resp.obj['flavor']) - self.flavor_action_controller.show(self.req, resp, '2') - self.assertEqual({'id': '0', 'os-flavor-access:is_public': False}, - resp.obj['flavor']) - - def test_detail(self): - resp = FakeResponse() - self.flavor_action_controller.detail(self.req, resp) - self.assertEqual([{'id': '0', 'os-flavor-access:is_public': True}, - {'id': '2', 'os-flavor-access:is_public': False}], - resp.obj['flavors']) - - def test_create(self): - resp = FakeResponse() - self.flavor_action_controller.create(self.req, {}, resp) - self.assertEqual({'id': '0', 'os-flavor-access:is_public': True}, - resp.obj['flavor']) - def test_add_tenant_access(self): def stub_add_flavor_access(context, flavor_id, projectid): self.assertEqual(3, flavor_id, "flavor_id") @@ -423,21 +401,6 @@ class FlavorAccessPolicyEnforcementV21(test.NoDBTestCase): "Policy doesn't allow %s to be performed." % rule_name, exc.format_message()) - def test_extend_create_policy_failed(self): - rule_name = "os_compute_api:os-flavor-access" - self.policy.set_rules({rule_name: "project:non_fake"}) - self.act_controller.create(self.req, None, None) - - def test_extend_show_policy_failed(self): - rule_name = "os_compute_api:os-flavor-access" - self.policy.set_rules({rule_name: "project:non_fake"}) - self.act_controller.show(self.req, None, None) - - def test_extend_detail_policy_failed(self): - rule_name = "os_compute_api:os-flavor-access" - self.policy.set_rules({rule_name: "project:non_fake"}) - self.act_controller.detail(self.req, None) - def test_index_policy_failed(self): rule_name = "os_compute_api:os-flavor-access" self.policy.set_rules({rule_name: "project:non_fake"}) diff --git a/nova/tests/unit/api/openstack/compute/test_flavor_manage.py b/nova/tests/unit/api/openstack/compute/test_flavor_manage.py index f5f68f45ceab..c7c356107c94 100644 --- a/nova/tests/unit/api/openstack/compute/test_flavor_manage.py +++ b/nova/tests/unit/api/openstack/compute/test_flavor_manage.py @@ -361,6 +361,7 @@ class FlavorManageTestV2_55(FlavorManageTestV21): ephemeral_gb=flavor['OS-FLV-EXT-DATA:ephemeral'], disabled=flavor['OS-FLV-DISABLED:disabled'], is_public=flavor['os-flavor-access:is_public'], + rxtx_factor=flavor['rxtx_factor'], description=flavor['description']) # Now null out the flavor description. flavor = self.controller._update( @@ -535,7 +536,8 @@ class FlavorManagerPolicyEnforcementV21(test.TestCase): default_flavor_policy = "os_compute_api:os-flavor-manage" create_flavor_policy = "os_compute_api:os-flavor-manage:create" rules = {default_flavor_policy: 'is_admin:True', - create_flavor_policy: 'rule:%s' % default_flavor_policy} + create_flavor_policy: 'rule:%s' % default_flavor_policy, + "os_compute_api:os-flavor-access": "project:non_fake"} self.policy.set_rules(rules) body = { "flavor": { diff --git a/nova/tests/unit/api/openstack/compute/test_flavor_rxtx.py b/nova/tests/unit/api/openstack/compute/test_flavor_rxtx.py deleted file mode 100644 index 52e2dd37165e..000000000000 --- a/nova/tests/unit/api/openstack/compute/test_flavor_rxtx.py +++ /dev/null @@ -1,66 +0,0 @@ -# Copyright 2012 Nebula, Inc. -# -# 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. - -from oslo_serialization import jsonutils - -from nova import test -from nova.tests.unit.api.openstack import fakes - - -class FlavorRxtxTestV21(test.NoDBTestCase): - content_type = 'application/json' - _prefix = "/v2/fake" - - def setUp(self): - super(FlavorRxtxTestV21, self).setUp() - fakes.stub_out_nw_api(self) - fakes.stub_out_flavor_get_all(self) - fakes.stub_out_flavor_get_by_flavor_id(self) - - def _make_request(self, url): - req = fakes.HTTPRequest.blank(url) - req.headers['Accept'] = self.content_type - res = req.get_response(self._get_app()) - return res - - def _get_app(self): - return fakes.wsgi_app_v21() - - def _get_flavor(self, body): - return jsonutils.loads(body).get('flavor') - - def _get_flavors(self, body): - return jsonutils.loads(body).get('flavors') - - def assertFlavorRxtx(self, flavor, rxtx): - self.assertEqual(flavor.get('rxtx_factor'), rxtx or u'') - - def test_show(self): - url = self._prefix + '/flavors/1' - res = self._make_request(url) - - self.assertEqual(res.status_int, 200) - self.assertFlavorRxtx(self._get_flavor(res.body), - fakes.FLAVORS['1'].rxtx_factor) - - def test_detail(self): - url = self._prefix + '/flavors/detail' - res = self._make_request(url) - - self.assertEqual(res.status_int, 200) - flavors = self._get_flavors(res.body) - self.assertFlavorRxtx(flavors[0], - fakes.FLAVORS['1'].rxtx_factor) - self.assertFlavorRxtx(flavors[1], - fakes.FLAVORS['2'].rxtx_factor) diff --git a/nova/tests/unit/api/openstack/compute/test_flavors.py b/nova/tests/unit/api/openstack/compute/test_flavors.py index 77b233f1a51b..0ffa8aec1467 100644 --- a/nova/tests/unit/api/openstack/compute/test_flavors.py +++ b/nova/tests/unit/api/openstack/compute/test_flavors.py @@ -91,6 +91,8 @@ class FlavorsTestV21(test.TestCase): "ram": fakes.FLAVORS['1'].memory_mb, "disk": fakes.FLAVORS['1'].root_gb, "vcpus": fakes.FLAVORS['1'].vcpus, + "os-flavor-access:is_public": True, + "rxtx_factor": 1.0, "links": [ { "rel": "self", @@ -121,6 +123,8 @@ class FlavorsTestV21(test.TestCase): "ram": fakes.FLAVORS['1'].memory_mb, "disk": fakes.FLAVORS['1'].root_gb, "vcpus": fakes.FLAVORS['1'].vcpus, + "os-flavor-access:is_public": True, + "rxtx_factor": 1.0, "links": [ { "rel": "self", @@ -237,6 +241,8 @@ class FlavorsTestV21(test.TestCase): "ram": fakes.FLAVORS['1'].memory_mb, "disk": fakes.FLAVORS['1'].root_gb, "vcpus": fakes.FLAVORS['1'].vcpus, + "os-flavor-access:is_public": True, + "rxtx_factor": 1.0, "links": [ { "rel": "self", @@ -360,6 +366,8 @@ class FlavorsTestV21(test.TestCase): "ram": fakes.FLAVORS['1'].memory_mb, "disk": fakes.FLAVORS['1'].root_gb, "vcpus": fakes.FLAVORS['1'].vcpus, + "os-flavor-access:is_public": True, + "rxtx_factor": 1.0, "links": [ { "rel": "self", @@ -379,6 +387,8 @@ class FlavorsTestV21(test.TestCase): "ram": fakes.FLAVORS['2'].memory_mb, "disk": fakes.FLAVORS['2'].root_gb, "vcpus": fakes.FLAVORS['2'].vcpus, + "os-flavor-access:is_public": True, + "rxtx_factor": '', "links": [ { "rel": "self", @@ -490,6 +500,8 @@ class FlavorsTestV21(test.TestCase): "ram": fakes.FLAVORS['2'].memory_mb, "disk": fakes.FLAVORS['2'].root_gb, "vcpus": fakes.FLAVORS['2'].vcpus, + "os-flavor-access:is_public": True, + "rxtx_factor": '', "links": [ { "rel": "self", @@ -515,6 +527,54 @@ class FlavorsTestV2_55(FlavorsTestV21): expect_description = True +class FlavorsPolicyEnforcementV21(test.NoDBTestCase): + + def setUp(self): + super(FlavorsPolicyEnforcementV21, self).setUp() + self.flavor_controller = flavors_v21.FlavorsController() + fakes.stub_out_flavor_get_by_flavor_id(self) + fakes.stub_out_flavor_get_all(self) + self.req = fakes.HTTPRequest.blank('') + + def test_show_flavor_access_policy_failed(self): + rule_name = "os_compute_api:os-flavor-access" + self.policy.set_rules({rule_name: "project:non_fake"}) + resp = self.flavor_controller.show(self.req, '1') + self.assertNotIn('os-flavor-access:is_public', resp['flavor']) + + def test_detail_flavor_access_policy_failed(self): + rule_name = "os_compute_api:os-flavor-access" + self.policy.set_rules({rule_name: "project:non_fake"}) + resp = self.flavor_controller.detail(self.req) + self.assertNotIn('os-flavor-access:is_public', resp['flavors'][0]) + + def test_show_flavor_rxtx_policy_failed(self): + rule_name = "os_compute_api:os-flavor-rxtx" + self.policy.set_rules({rule_name: "project:non_fake"}) + resp = self.flavor_controller.show(self.req, '1') + self.assertNotIn('rxtx_factor', resp['flavor']) + + def test_detail_flavor_rxtx_policy_failed(self): + rule_name = "os_compute_api:os-flavor-rxtx" + self.policy.set_rules({rule_name: "project:non_fake"}) + resp = self.flavor_controller.detail(self.req) + self.assertNotIn('rxtx_factor', resp['flavors'][0]) + + def test_create_flavor_extended_policy_failed(self): + rules = {"os_compute_api:os-flavor-rxtx": "project:non_fake", + "os_compute_api:os-flavor-access": "project:non_fake"} + self.policy.set_rules(rules) + resp = self.flavor_controller.detail(self.req) + self.assertNotIn('rxtx_factor', resp['flavors'][0]) + + def test_update_flavor_extended_policy_failed(self): + rules = {"os_compute_api:os-flavor-rxtx": "project:non_fake", + "os_compute_api:os-flavor-access": "project:non_fake"} + self.policy.set_rules(rules) + resp = self.flavor_controller.detail(self.req) + self.assertNotIn('rxtx_factor', resp['flavors'][0]) + + class DisabledFlavorsWithRealDBTestV21(test.TestCase): """Tests that disabled flavors should not be shown nor listed.""" Controller = flavors_v21.FlavorsController