openstack
/
nova
Code Issues Proposed changes

Be more tolerant of keystone catalog configuration 

The previous attempt at validating against keystone assumed that users
had an unversioned endpoint in their catalog. Most do not. This uses
the keystoneauth discovery magic to do the right thing whether or not
they have the versioned or unversioned endpoint, as long as they have
a working v3 installation.

It is also much more verbose on log messages when things go wrong.

Change-Id: I509d406ca1f1f7b064aaca88645ad17685827267
Related-Bug: #1667679
Closes-Bug: #1693228
This commit is contained in:
Sean Dague 2017-02-24 13:56:03 -05:00
parent 6b38344496
commit 4a2009a6fa
4 changed files with 107 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
nova/api/openstack/identity.py
View File

@ -30,10 +30,22 @@ def verify_project_id(context, project_id):
"""
sess = session.Session(auth=context.get_auth_plugin())
failure = webob.exc.HTTPBadRequest(
explanation=_("Project ID %s is not a valid project.") %
project_id)
try:
resp = sess.get('/v3/projects/%s' % project_id,
endpoint_filter={'service_type': 'identity'},
resp = sess.get('/projects/%s' % project_id,
endpoint_filter={
'service_type': 'identity',
'version': (3, 0)
},
raise_exc=False)
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
except kse.ClientException:
# something is wrong, like there isn't a keystone v3 endpoint,
# we'll take the pass and default to everything being ok.
@ -45,9 +57,7 @@ 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 webob.exc.HTTPBadRequest(
explanation=_("Project ID %s is not a valid project.") %
project_id)
raise failure
elif resp.status_code == 403:
# we don't have enough permission to verify this, so default
# to "it's ok".

6
nova/tests/unit/api/openstack/compute/test_flavor_access.py
View File

@ -136,6 +136,12 @@ class FlavorAccessTestV21(test.NoDBTestCase):
def setUp(self):
super(FlavorAccessTestV21, self).setUp()
self.flavor_controller = flavors_api.FlavorsController()
# We need to stub out verify_project_id so that it doesn't
# generate an EndpointNotFound exception and result in a
# server error.
self.stub_out('nova.api.openstack.identity.verify_project_id',
lambda ctx, project_id: True)
self.req = FakeRequest()
self.req.environ = {"nova.context": context.RequestContext('fake_user',
'fake')}

14
nova/tests/unit/api/openstack/compute/test_quotas.py
View File

@ -41,6 +41,14 @@ def quota_set(id, include_server_group_quotas=True):
class BaseQuotaSetsTest(test.TestCase):
def setUp(self):
super(BaseQuotaSetsTest, self).setUp()
# We need to stub out verify_project_id so that it doesn't
# generate an EndpointNotFound exception and result in a
# server error.
self.stub_out('nova.api.openstack.identity.verify_project_id',
lambda ctx, project_id: True)
def get_delete_status_int(self, res):
# NOTE: on v2.1, http status code is set as wsgi_code of API
# method instead of status_int in a response object.
@ -531,6 +539,12 @@ class QuotaSetsTestV236(test.NoDBTestCase):
def setUp(self):
super(QuotaSetsTestV236, self).setUp()
# We need to stub out verify_project_id so that it doesn't
# generate an EndpointNotFound exception and result in a
# server error.
self.stub_out('nova.api.openstack.identity.verify_project_id',
lambda ctx, project_id: True)
self.flags(enable_network_quota=True)
tenant_networks._register_network_quota()
self.old_req = fakes.HTTPRequest.blank('', version='2.1')

81
nova/tests/unit/test_identity.py
View File

@ -47,46 +47,109 @@ class FakeResponse(object):
class IdentityValidationTest(test.NoDBTestCase):
"""Unit tests for our validation of keystone projects.
There are times when Nova stores keystone project_id and user_id
in our database as strings. Until the Pike release none of this
data was validated, so it was very easy for adminstrators to think
they were adjusting quota for a project (by name) when instead
they were just inserting keys in a database that would not get used.
This is only tested in unit tests through mocking out keystoneauth
responses because a functional test would need a real keystone or
keystone simulator.
The functional code works by using the existing keystone
credentials and trying to make a /v3/projects/{id} get call. It
will return a 403 if the user doesn't have enough permissions to
ask about other projects, a 404 if it does and that project does
not exist.
"""
@mock.patch('keystoneauth1.session.Session.get')
def test_good_id(self, get):
"""Test response 200.
This indicates we have permissions, and we have definitively
found the project exists.
"""
get.return_value = FakeResponse(200)
self.assertTrue(identity.verify_project_id(mock.MagicMock(), "foo"))
get.assert_called_once_with(
'/v3/projects/foo',
endpoint_filter={'service_type': 'identity'},
'/projects/foo',
endpoint_filter={'service_type': 'identity', 'version': (3, 0)},
raise_exc=False)
@mock.patch('keystoneauth1.session.Session.get')
def test_no_project(self, get):
"""Test response 404.
This indicates that we have permissions, and we have
definitively found the project does not exist.
"""
get.return_value = FakeResponse(404)
self.assertRaises(webob.exc.HTTPBadRequest,
identity.verify_project_id,
mock.MagicMock(), "foo")
get.assert_called_once_with(
'/v3/projects/foo',
endpoint_filter={'service_type': 'identity'},
'/projects/foo',
endpoint_filter={'service_type': 'identity', 'version': (3, 0)},
raise_exc=False)
@mock.patch('keystoneauth1.session.Session.get')
def test_unknown_id(self, get):
"""Test response 403.
This indicates we don't have permissions. We fail open here
and assume the project exists.
"""
get.return_value = FakeResponse(403)
self.assertTrue(identity.verify_project_id(mock.MagicMock(), "foo"))
get.assert_called_once_with(
'/v3/projects/foo',
endpoint_filter={'service_type': 'identity'},
'/projects/foo',
endpoint_filter={'service_type': 'identity', 'version': (3, 0)},
raise_exc=False)
@mock.patch('keystoneauth1.session.Session.get')
def test_unknown_error(self, get):
"""Test some other return from keystone.
If we got anything else, something is wrong on the keystone
side. We don't want to fail on our side.
"""
get.return_value = FakeResponse(500, "Oh noes!")
self.assertTrue(identity.verify_project_id(mock.MagicMock(), "foo"))
get.assert_called_once_with(
'/v3/projects/foo',
endpoint_filter={'service_type': 'identity'},
'/projects/foo',
endpoint_filter={'service_type': 'identity', 'version': (3, 0)},
raise_exc=False)
@mock.patch('keystoneauth1.session.Session.get')
def test_early_fail(self, get):
get.side_effect = kse.EndpointNotFound()
"""Test if we get a keystoneauth exception.
If we get a random keystoneauth exception, fall back and
assume the project exists.
"""
get.side_effect = kse.ConnectionError()
self.assertTrue(identity.verify_project_id(mock.MagicMock(), "foo"))
@mock.patch('keystoneauth1.session.Session.get')
def test_wrong_version(self, get):
"""Test endpoint not found.
EndpointNotFound will be made when the keystone v3 API is not
found in the service catalog, or if the v2.0 endpoint was
registered as the root endpoint. We treat this the same as 404.
"""
get.side_effect = kse.EndpointNotFound()
self.assertRaises(webob.exc.HTTPBadRequest,
identity.verify_project_id,
mock.MagicMock(), "foo")