diff options
-rw-r--r-- | nova/api/openstack/compute/flavor_access.py | 9 | ||||
-rw-r--r-- | nova/api/openstack/identity.py | 22 | ||||
-rw-r--r-- | nova/tests/unit/api/openstack/compute/test_flavor_access.py | 25 |
3 files changed, 45 insertions, 11 deletions
diff --git a/nova/api/openstack/compute/flavor_access.py b/nova/api/openstack/compute/flavor_access.py index e17e6f0ddc..fc8df15db5 100644 --- a/nova/api/openstack/compute/flavor_access.py +++ b/nova/api/openstack/compute/flavor_access.py @@ -93,7 +93,14 @@ class FlavorActionController(wsgi.Controller): vals = body['removeTenantAccess'] tenant = vals['tenant'] - identity.verify_project_id(context, tenant) + # It doesn't really matter if project exists or not: we can delete + # it from flavor's access list in both cases. + try: + identity.verify_project_id(context, tenant) + except webob.exc.HTTPBadRequest as identity_exc: + msg = "Project ID %s is not a valid project." % tenant + if msg not in identity_exc.explanation: + raise # NOTE(gibi): We have to load a flavor from the db here as # flavor.remove_access() will try to emit a notification and that needs diff --git a/nova/api/openstack/identity.py b/nova/api/openstack/identity.py index 7ffc623fed..15ec884aea 100644 --- a/nova/api/openstack/identity.py +++ b/nova/api/openstack/identity.py @@ -27,24 +27,27 @@ def verify_project_id(context, project_id): """verify that a project_id exists. This attempts to verify that a project id exists. If it does not, - an HTTPBadRequest is emitted. + an HTTPBadRequest is emitted. Also HTTPBadRequest is emitted + if Keystone identity service version 3.0 is not found. """ adap = utils.get_ksa_adapter( 'identity', ksa_auth=context.get_auth_plugin(), min_version=(3, 0), max_version=(3, 'latest')) - failure = webob.exc.HTTPBadRequest( - explanation=_("Project ID %s is not a valid project.") % - project_id) try: resp = adap.get('/projects/%s' % project_id) except kse.EndpointNotFound: LOG.error( - "Keystone identity service version 3.0 was not found. This might " - "be because your endpoint points to the v2.0 versioned endpoint " - "which is not supported. Please fix this.") - raise failure + "Keystone identity service version 3.0 was not found. This " + "might be caused by Nova misconfiguration or Keystone " + "problems.") + msg = _("Nova was unable to find Keystone service endpoint.") + # TODO(astupnik). It may be reasonable to switch to HTTP 503 + # (HTTP Service Unavailable) instead of HTTP Bad Request here. + # If proper Keystone servie is inaccessible, then technially + # this is a server side error and not an error in Nova. + raise webob.exc.HTTPBadRequest(explanation=msg) except kse.ClientException: # something is wrong, like there isn't a keystone v3 endpoint, # or nova isn't configured for the interface to talk to it; @@ -57,7 +60,8 @@ def verify_project_id(context, project_id): return True elif resp.status_code == 404: # we got access, and we know this project is not there - raise failure + msg = _("Project ID %s is not a valid project.") % project_id + raise webob.exc.HTTPBadRequest(explanation=msg) elif resp.status_code == 403: # we don't have enough permission to verify this, so default # to "it's ok". 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 0581a47c84..ea9ca2f632 100644 --- a/nova/tests/unit/api/openstack/compute/test_flavor_access.py +++ b/nova/tests/unit/api/openstack/compute/test_flavor_access.py @@ -353,14 +353,37 @@ class FlavorAccessTestV21(test.NoDBTestCase): mock_verify.assert_called_once_with( req.environ['nova.context'], 'proj2') + @mock.patch('nova.objects.Flavor.remove_access') @mock.patch('nova.api.openstack.identity.verify_project_id', side_effect=exc.HTTPBadRequest( explanation="Project ID proj2 is not a valid project.")) - def test_remove_tenant_access_with_invalid_tenant(self, mock_verify): + def test_remove_tenant_access_with_invalid_tenant(self, + mock_verify, + mock_remove_access): """Tests the case that the tenant does not exist in Keystone.""" req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action', use_admin_context=True) body = {'removeTenantAccess': {'tenant': 'proj2'}} + + self.flavor_action_controller._remove_tenant_access( + req, '2', body=body) + mock_verify.assert_called_once_with( + req.environ['nova.context'], 'proj2') + mock_remove_access.assert_called_once_with('proj2') + + @mock.patch('nova.api.openstack.identity.verify_project_id', + side_effect=exc.HTTPBadRequest( + explanation="Nova was unable to find Keystone " + "service endpoint.")) + def test_remove_tenant_access_missing_keystone_endpoint(self, + mock_verify): + """Tests the case that Keystone identity service endpoint + version 3.0 was not found. + """ + req = fakes.HTTPRequest.blank(self._prefix + '/flavors/2/action', + use_admin_context=True) + body = {'removeTenantAccess': {'tenant': 'proj2'}} + self.assertRaises(exc.HTTPBadRequest, self.flavor_action_controller._remove_tenant_access, req, '2', body=body) |