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
793bcc07b9 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

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"
         }

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
 
 

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())

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.

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"
         }

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):
 

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": [