From 6abb88befe2914040e3307b0bc5dfd13683db8b2 Mon Sep 17 00:00:00 2001 From: jichenjc Date: Sun, 21 Sep 2014 03:16:32 +0800 Subject: [PATCH] Check flavor type before add tenant access Currently we allow tenant access for public flavor, however, we can't query it after that because flavor is public. This patch adds check for add access function to raise exception if the flavor is public. Also, a nit change is use methods introduced in 793bcc07b9ae41b3de2c146cf69117632c88e27b to get flavor. APIImpact: Adds new 2.7 API microversion due to new error condition in flavor access API Implements blueprint check-flavor-type-before-add-tenant Closes-Bug: #1361476 Change-Id: I461175e9969a0dd5b2b7ef75ea7d9f36f3a306d0 --- .../versions/versions-get-resp.json | 2 +- nova/api/openstack/api_version_request.py | 3 +- .../compute/plugins/v3/flavor_access.py | 9 +++- .../openstack/rest_api_version_history.rst | 6 +++ .../versions/versions-get-resp.json.tpl | 2 +- .../compute/contrib/test_flavor_access.py | 43 +++++++++++++++++++ .../api/openstack/compute/test_versions.py | 4 +- 7 files changed, 63 insertions(+), 6 deletions(-) diff --git a/doc/api_samples/versions/versions-get-resp.json b/doc/api_samples/versions/versions-get-resp.json index 34503af2b46b..9f9120a2a7bc 100644 --- a/doc/api_samples/versions/versions-get-resp.json +++ b/doc/api_samples/versions/versions-get-resp.json @@ -22,7 +22,7 @@ } ], "status": "CURRENT", - "version": "2.6", + "version": "2.7", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/nova/api/openstack/api_version_request.py b/nova/api/openstack/api_version_request.py index 21b80a4d0d85..4160950e197e 100644 --- a/nova/api/openstack/api_version_request.py +++ b/nova/api/openstack/api_version_request.py @@ -45,6 +45,7 @@ REST_API_VERSION_HISTORY = """REST API Version History: * 2.4 - Exposes reserved field in os-fixed-ips. * 2.5 - Allow server search option ip6 for non-admin * 2.6 - Consolidate the APIs for getting remote consoles + * 2.7 - Check flavor type before add tenant access. """ # The minimum and maximum versions of the API supported @@ -53,7 +54,7 @@ REST_API_VERSION_HISTORY = """REST API Version History: # Note(cyeoh): This only applies for the v2.1 API once microversions # support is fully merged. It does not affect the V2 API. _MIN_API_VERSION = "2.1" -_MAX_API_VERSION = "2.6" +_MAX_API_VERSION = "2.7" DEFAULT_API_VERSION = _MIN_API_VERSION diff --git a/nova/api/openstack/compute/plugins/v3/flavor_access.py b/nova/api/openstack/compute/plugins/v3/flavor_access.py index 8456fc00e973..4ca9b627c996 100644 --- a/nova/api/openstack/compute/plugins/v3/flavor_access.py +++ b/nova/api/openstack/compute/plugins/v3/flavor_access.py @@ -17,6 +17,7 @@ import webob +from nova.api.openstack import api_version_request from nova.api.openstack import common from nova.api.openstack.compute.schemas.v3 import flavor_access from nova.api.openstack import extensions @@ -103,8 +104,14 @@ class FlavorActionController(wsgi.Controller): vals = body['addTenantAccess'] tenant = vals['tenant'] - flavor = objects.Flavor(context=context, flavorid=id) + flavor = common.get_flavor(context, id) + try: + req_ver = req.api_version_request + if req_ver >= api_version_request.APIVersionRequest("2.7"): + if flavor.is_public: + exp = _("Can not add access to a public flavor.") + raise webob.exc.HTTPConflict(explanation=exp) flavor.add_access(tenant) except exception.FlavorNotFound as e: raise webob.exc.HTTPNotFound(explanation=e.format_message()) diff --git a/nova/api/openstack/rest_api_version_history.rst b/nova/api/openstack/rest_api_version_history.rst index e221144d54ce..281250217d21 100644 --- a/nova/api/openstack/rest_api_version_history.rst +++ b/nova/api/openstack/rest_api_version_history.rst @@ -90,3 +90,9 @@ user documentation. The old APIs 'os-getVNCConsole', 'os-getSPICEConsole', 'os-getSerialConsole' and 'os-getRDPConsole' are removed. + +2.7 +--- + + Check the ``is_public`` attribute of a flavor before adding tenant access + to it. Reject the request with HTTPConflict error. diff --git a/nova/tests/functional/api_samples/versions/versions-get-resp.json.tpl b/nova/tests/functional/api_samples/versions/versions-get-resp.json.tpl index 34503af2b46b..9f9120a2a7bc 100644 --- a/nova/tests/functional/api_samples/versions/versions-get-resp.json.tpl +++ b/nova/tests/functional/api_samples/versions/versions-get-resp.json.tpl @@ -22,7 +22,7 @@ } ], "status": "CURRENT", - "version": "2.6", + "version": "2.7", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z" } diff --git a/nova/tests/unit/api/openstack/compute/contrib/test_flavor_access.py b/nova/tests/unit/api/openstack/compute/contrib/test_flavor_access.py index a73e4b7ecade..6e9e7d0e6060 100644 --- a/nova/tests/unit/api/openstack/compute/contrib/test_flavor_access.py +++ b/nova/tests/unit/api/openstack/compute/contrib/test_flavor_access.py @@ -15,6 +15,7 @@ import datetime +import mock import six from webob import exc @@ -296,6 +297,16 @@ class FlavorAccessTestV21(test.NoDBTestCase): result = add_access(req, '3', body=body) self.assertEqual(result, expected) + @mock.patch('nova.objects.Flavor.get_by_flavor_id', + side_effect=exception.FlavorNotFound(flavor_id='1')) + def test_add_tenant_access_with_flavor_not_found(self, mock_get): + body = {'addTenantAccess': {'tenant': 'proj2'}} + req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action', + use_admin_context=True) + add_access = self._get_add_access() + self.assertRaises(exc.HTTPNotFound, + add_access, req, '2', body=body) + def test_add_tenant_access_with_no_tenant(self): req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action', use_admin_context=True) @@ -329,6 +340,15 @@ class FlavorAccessTestV21(test.NoDBTestCase): self.assertRaises(exc.HTTPNotFound, remove_access, self.req, '3', body=body) + def test_add_tenant_access_is_public(self): + body = {'addTenantAccess': {'tenant': 'proj2'}} + req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action', + use_admin_context=True) + req.api_version_request = api_version.APIVersionRequest('2.7') + add_access = self._get_add_access() + self.assertRaises(exc.HTTPConflict, + add_access, req, '1', body=body) + def test_delete_tenant_access_with_no_tenant(self): req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action', use_admin_context=True) @@ -374,6 +394,29 @@ class FlavorAccessTestV20(FlavorAccessTestV21): self.flavor_access_controller.index, req, 'fake') + @mock.patch('nova.objects.Flavor.add_access') + def test_add_tenant_access_is_public(self, mock_add): + # V2 don't test public before add access + expected = {'flavor_access': + [{'flavor_id': '3', 'tenant_id': 'proj3'}]} + body = {'addTenantAccess': {'tenant': 'proj2'}} + req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action', + use_admin_context=True) + + add_access = self._get_add_access() + result = add_access(req, '3', body=body) + mock_add.assert_called_once_with('proj2') + self.assertEqual(result, expected) + + @mock.patch('nova.objects.Flavor.add_access') + def test_add_tenant_access_with_flavor_not_found(self, mock_add): + body = {'addTenantAccess': {'tenant': 'proj2'}} + req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action', + use_admin_context=True) + add_access = self._get_add_access() + add_access(req, '1000', body=body) + mock_add.assert_called_once_with('proj2') + class FlavorAccessPolicyEnforcementV21(test.NoDBTestCase): diff --git a/nova/tests/unit/api/openstack/compute/test_versions.py b/nova/tests/unit/api/openstack/compute/test_versions.py index d54e723c5b47..3bf24f6246b7 100644 --- a/nova/tests/unit/api/openstack/compute/test_versions.py +++ b/nova/tests/unit/api/openstack/compute/test_versions.py @@ -65,7 +65,7 @@ EXP_VERSIONS = { "v2.1": { "id": "v2.1", "status": "CURRENT", - "version": "2.6", + "version": "2.7", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z", "links": [ @@ -114,7 +114,7 @@ class VersionsTestV20(test.NoDBTestCase): { "id": "v2.1", "status": "CURRENT", - "version": "2.6", + "version": "2.7", "min_version": "2.1", "updated": "2013-07-23T11:33:21Z", "links": [