openstack/cinder
Code Issues Proposed changes

Modify/Move project validation methods to api_utils

Browse Source 
We currently only validate the existence of a project on
the default types code so that we cannot set it for non
existing projects, but there are other places where we would
benefit from having this validation, such as volume type access,
setting quotas.
To achieve this we are moving methods from quota_utils and default_types
to api_utils.
This patch also modifies the method get_project_hierarchy to get_project
and removes related hierarchy tests since with the removal of nested quota
driver we don't require the hierarchy properties anymore.

Related-Bug: #1638804

Change-Id: I20f8e500f583eb85f24a4c36f344c11c7face06c
This commit is contained in:
Rajat Dhasmana 2020-09-17 10:16:50 +00:00
parent 393c2e4ad9
commit dcf66a96a4
9 changed files with 152 additions and 231 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

91
cinder/api/api_utils.py
View File

@ -10,13 +10,23 @@
# License for the specific language governing permissions and limitations
# under the License.
from keystoneauth1 import exceptions as ks_exc
from keystoneauth1 import identity
from keystoneauth1 import loading as ka_loading
from keystoneclient import client
from oslo_config import cfg
from oslo_log import log as logging
from oslo_utils import strutils
import webob
from webob import exc
from cinder import exception
from cinder.i18n import _
CONF = cfg.CONF
CONF.import_group('keystone_authtoken',
'keystonemiddleware.auth_token.__init__')
LOG = logging.getLogger(__name__)
@ -142,3 +152,84 @@ def walk_class_hierarchy(clazz, encountered=None):
for subsubclass in walk_class_hierarchy(subclass, encountered):
yield subsubclass
yield subclass
def _keystone_client(context, version=(3, 0)):
"""Creates and returns an instance of a generic keystone client.
:param context: The request context
:param version: version of Keystone to request
:return: keystoneclient.client.Client object
"""
if context.system_scope is not None:
auth_plugin = identity.Token(
auth_url=CONF.keystone_authtoken.auth_url,
token=context.auth_token,
system_scope=context.system_scope
)
elif context.domain_id is not None:
auth_plugin = identity.Token(
auth_url=CONF.keystone_authtoken.auth_url,
token=context.auth_token,
domain_id=context.domain_id
)
elif context.project_id is not None:
auth_plugin = identity.Token(
auth_url=CONF.keystone_authtoken.auth_url,
token=context.auth_token,
project_id=context.project_id
)
else:
# We're dealing with an unscoped token from keystone that doesn't
# carry any authoritative power outside of the user simplify proving
# they know their own password. This token isn't associated with any
# authorization target (e.g., system, domain, or project).
auth_plugin = context.get_auth_plugin()
client_session = ka_loading.session.Session().load_from_options(
auth=auth_plugin,
insecure=CONF.keystone_authtoken.insecure,
cacert=CONF.keystone_authtoken.cafile,
key=CONF.keystone_authtoken.keyfile,
cert=CONF.keystone_authtoken.certfile,
split_loggers=CONF.service_user.split_loggers)
return client.Client(auth_url=CONF.keystone_authtoken.auth_url,
session=client_session, version=version)
class GenericProjectInfo(object):
"""Abstraction layer for Keystone V2 and V3 project objects"""
def __init__(self, project_id, project_keystone_api_version,
domain_id=None, name=None, description=None):
self.id = project_id
self.keystone_api_version = project_keystone_api_version
self.domain_id = domain_id
self.name = name
self.description = description
def get_project(context, project_id):
"""Method to verify project exists in keystone"""
keystone = _keystone_client(context)
generic_project = GenericProjectInfo(project_id, keystone.version)
project = keystone.projects.get(project_id)
generic_project.domain_id = project.domain_id
generic_project.name = project.name
generic_project.description = project.description
return generic_project
def validate_project_and_authorize(context, project_id, policy_check=None,
validate_only=False):
try:
target_project = get_project(context, project_id)
if not validate_only:
target_project = {'project_id': target_project.id}
context.authorize(policy_check, target=target_project)
except ks_exc.http.NotFound:
explanation = _("Project with id %s not found." % project_id)
raise exc.HTTPNotFound(explanation=explanation)
except exception.NotAuthorized:
explanation = _("You are not authorized to perform this "
"operation.")
raise exc.HTTPForbidden(explanation=explanation)

25
cinder/api/v3/default_types.py
View File

@ -16,9 +16,9 @@
"""The resource filters api."""
from http import HTTPStatus
from keystoneauth1 import exceptions as ks_exc
from webob import exc
from cinder.api import api_utils as utils
from cinder.api import microversions as mv
from cinder.api.openstack import wsgi
from cinder.api.schemas import default_types
@ -29,7 +29,6 @@ from cinder import exception
from cinder.i18n import _
from cinder import objects
from cinder.policies import default_types as policy
from cinder import quota_utils
class DefaultTypesController(wsgi.Controller):
@ -37,22 +36,6 @@ class DefaultTypesController(wsgi.Controller):
_view_builder_class = default_types_view.ViewBuilder
def _validate_project_and_authorize(self, context, project_id,
policy_check):
try:
target_project = quota_utils.get_project_hierarchy(context,
project_id)
target_project = {'project_id': target_project.id,
'domain_id': target_project.domain_id}
context.authorize(policy_check, target=target_project)
except ks_exc.http.NotFound:
explanation = _("Project with id %s not found." % project_id)
raise exc.HTTPNotFound(explanation=explanation)
except exception.NotAuthorized:
explanation = _("You are not authorized to perform this "
"operation.")
raise exc.HTTPForbidden(explanation=explanation)
@wsgi.response(HTTPStatus.OK)
@wsgi.Controller.api_version(mv.DEFAULT_TYPE_OVERRIDES)
@validation.schema(default_types.create_or_update)
@ -63,7 +46,7 @@ class DefaultTypesController(wsgi.Controller):
project_id = id
volume_type_id = body['default_type']['volume_type']
self._validate_project_and_authorize(context, project_id,
utils.validate_project_and_authorize(context, project_id,
policy.CREATE_UPDATE_POLICY)
try:
volume_type_id = objects.VolumeType.get_by_name_or_id(
@ -85,7 +68,7 @@ class DefaultTypesController(wsgi.Controller):
context = req.environ['cinder.context']
project_id = id
self._validate_project_and_authorize(context, project_id,
utils.validate_project_and_authorize(context, project_id,
policy.GET_POLICY)
default_type = db.project_default_volume_type_get(context, project_id)
if not default_type:
@ -117,7 +100,7 @@ class DefaultTypesController(wsgi.Controller):
context = req.environ['cinder.context']
project_id = id
self._validate_project_and_authorize(context, project_id,
utils.validate_project_and_authorize(context, project_id,
policy.DELETE_POLICY)
db.project_default_volume_type_unset(context, id)

96
cinder/quota_utils.py
View File

@ -12,9 +12,6 @@
# 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 keystoneauth1 import identity
from keystoneauth1 import loading as ka_loading
from keystoneclient import client
from oslo_config import cfg
from oslo_log import log as logging
@ -27,23 +24,6 @@ CONF.import_group('keystone_authtoken',
LOG = logging.getLogger(__name__)
class GenericProjectInfo(object):
"""Abstraction layer for Keystone V2 and V3 project objects"""
def __init__(self, project_id, project_keystone_api_version,
project_parent_id=None,
project_subtree=None,
project_parent_tree=None,
is_admin_project=False,
domain_id=None):
self.id = project_id
self.domain_id = domain_id
self.keystone_api_version = project_keystone_api_version
self.parent_id = project_parent_id
self.subtree = project_subtree
self.parents = project_parent_tree
self.is_admin_project = is_admin_project
def get_volume_type_reservation(ctxt, volume, type_id,
reserve_vol_type_only=False,
negative=False):
@ -101,82 +81,6 @@ def _filter_domain_id_from_parents(domain_id, tree):
return new_tree
def get_project_hierarchy(context, project_id, subtree_as_ids=False,
parents_as_ids=False, is_admin_project=False):
"""A Helper method to get the project hierarchy.
Along with hierarchical multitenancy in keystone API v3, projects can be
hierarchically organized. Therefore, we need to know the project
hierarchy, if any, in order to do default volume type operations properly.
"""
keystone = _keystone_client(context)
generic_project = GenericProjectInfo(project_id, keystone.version)
if keystone.version == 'v3':
project = keystone.projects.get(project_id,
subtree_as_ids=subtree_as_ids,
parents_as_ids=parents_as_ids)
generic_project.parent_id = None
generic_project.domain_id = project.domain_id
if project.parent_id != project.domain_id:
generic_project.parent_id = project.parent_id
generic_project.subtree = (
project.subtree if subtree_as_ids else None)
generic_project.parents = None
if parents_as_ids:
generic_project.parents = _filter_domain_id_from_parents(
project.domain_id, project.parents)
generic_project.is_admin_project = is_admin_project
return generic_project
def _keystone_client(context, version=(3, 0)):
"""Creates and returns an instance of a generic keystone client.
:param context: The request context
:param version: version of Keystone to request
:return: keystoneclient.client.Client object
"""
if context.system_scope is not None:
auth_plugin = identity.Token(
auth_url=CONF.keystone_authtoken.auth_url,
token=context.auth_token,
system_scope=context.system_scope
)
elif context.domain_id is not None:
auth_plugin = identity.Token(
auth_url=CONF.keystone_authtoken.auth_url,
token=context.auth_token,
domain_id=context.domain_id
)
elif context.project_id is not None:
auth_plugin = identity.Token(
auth_url=CONF.keystone_authtoken.auth_url,
token=context.auth_token,
project_id=context.project_id
)
else:
# We're dealing with an unscoped token from keystone that doesn't
# carry any authoritative power outside of the user simplify proving
# they know their own password. This token isn't associated with any
# authorization target (e.g., system, domain, or project).
auth_plugin = context.get_auth_plugin()
client_session = ka_loading.session.Session().load_from_options(
auth=auth_plugin,
insecure=CONF.keystone_authtoken.insecure,
cacert=CONF.keystone_authtoken.cafile,
key=CONF.keystone_authtoken.keyfile,
cert=CONF.keystone_authtoken.certfile,
split_loggers=CONF.service_user.split_loggers)
return client.Client(auth_url=CONF.keystone_authtoken.auth_url,
session=client_session, version=version)
OVER_QUOTA_RESOURCE_EXCEPTIONS = {'snapshots': exception.SnapshotLimitExceeded,
'backups': exception.BackupLimitExceeded,
'volumes': exception.VolumeLimitExceeded,

13
cinder/tests/functional/test_default_types.py
View File

@ -35,7 +35,7 @@ class DefaultVolumeTypesTest(functional_helpers._FunctionalTestBase):
_keystone_client.version = 'v3'
_keystone_client.projects.get.side_effect = self._get_project
_keystone_client_get = mock.patch(
'cinder.quota_utils._keystone_client',
'cinder.api.api_utils._keystone_client',
lambda *args, **kwargs: _keystone_client)
_keystone_client_get.start()
self.addCleanup(_keystone_client_get.stop)
@ -44,14 +44,11 @@ class DefaultVolumeTypesTest(functional_helpers._FunctionalTestBase):
return self.project
class FakeProject(object):
_dom_id = uuid.uuid4().hex
def __init__(self, parent_id=None):
def __init__(self, name=None):
self.id = uuid.uuid4().hex
self.parent_id = parent_id
self.domain_id = self._dom_id
self.subtree = None
self.parents = None
self.name = name
self.description = 'fake project description'
self.domain_id = 'default'
@mock.patch.object(context.RequestContext, 'authorize')
def test_default_type_set(self, mock_authorize):

2
cinder/tests/unit/api/contrib/test_quotas.py
View File

@ -99,7 +99,7 @@ class QuotaSetsControllerTestBase(test.TestCase):
self.req.environ['cinder.context'].project_id = uuid.uuid4().hex
get_patcher = mock.patch('cinder.quota_utils.get_project_hierarchy',
get_patcher = mock.patch('cinder.api.api_utils.get_project',
self._get_project)
get_patcher.start()
self.addCleanup(get_patcher.stop)

2
cinder/tests/unit/api/v3/test_default_types.py
View File

@ -79,7 +79,7 @@ class DefaultVolumeTypesApiTest(test.TestCase):
self.type2 = self._create_volume_type(
self.ctxt, 'volume_type2')
get_patcher = mock.patch('cinder.quota_utils.get_project_hierarchy',
get_patcher = mock.patch('cinder.api.api_utils.get_project',
self._get_project)
get_patcher.start()
self.addCleanup(get_patcher.stop)

16
cinder/tests/unit/policies/test_default_volume_types.py
View File

@ -15,6 +15,7 @@
from http import HTTPStatus
from unittest import mock
import uuid
from cinder.api import microversions as mv
from cinder import db
@ -38,7 +39,7 @@ class DefaultVolumeTypesPolicyTests(test_base.CinderPolicyTests):
_keystone_client.version = 'v3'
_keystone_client.projects.get.side_effect = self._get_project
_keystone_client_get = mock.patch(
'cinder.quota_utils._keystone_client',
'cinder.api.api_utils._keystone_client',
lambda *args, **kwargs: _keystone_client)
_keystone_client_get.start()
self.addCleanup(_keystone_client_get.stop)
@ -47,14 +48,11 @@ class DefaultVolumeTypesPolicyTests(test_base.CinderPolicyTests):
return self.project
class FakeProject(object):
_dom_id = fake_constants.DOMAIN_ID
def __init__(self, parent_id=None):
self.id = fake_constants.PROJECT_ID
self.parent_id = parent_id
self.domain_id = self._dom_id
self.subtree = None
self.parents = None
def __init__(self, name=None):
self.id = uuid.uuid4().hex
self.name = name
self.description = 'fake project description'
self.domain_id = 'default'
def test_system_admin_can_set_default(self):
system_admin_context = self.system_admin_context

100
cinder/tests/unit/test_quota_utils.py
View File

@ -18,6 +18,7 @@ from unittest import mock
from oslo_config import cfg
from oslo_config import fixture as config_fixture
from cinder.api import api_utils
from cinder import context
from cinder import exception
from cinder import quota_utils
@ -29,13 +30,6 @@ CONF = cfg.CONF
class QuotaUtilsTest(test.TestCase):
class FakeProject(object):
def __init__(self, id='foo', parent_id=None):
self.id = id
self.parent_id = parent_id
self.subtree = None
self.parents = None
self.domain_id = 'default'
def setUp(self):
super(QuotaUtilsTest, self).setUp()
@ -49,7 +43,7 @@ class QuotaUtilsTest(test.TestCase):
@mock.patch('keystoneauth1.session.Session')
def test_keystone_client_instantiation(self, ksclient_session,
ksclient_class):
quota_utils._keystone_client(self.context)
api_utils._keystone_client(self.context)
ksclient_class.assert_called_once_with(auth_url=self.auth_url,
session=ksclient_session(),
version=(3, 0))
@ -61,7 +55,7 @@ class QuotaUtilsTest(test.TestCase):
self, ks_token, ksclient_session, ksclient_class):
system_context = context.RequestContext(
'fake_user', 'fake_proj_id', system_scope='all')
quota_utils._keystone_client(system_context)
api_utils._keystone_client(system_context)
ks_token.assert_called_once_with(
auth_url=self.auth_url, token=system_context.auth_token,
system_scope=system_context.system_scope)
@ -73,7 +67,7 @@ class QuotaUtilsTest(test.TestCase):
self, ks_token, ksclient_session, ksclient_class):
domain_context = context.RequestContext(
'fake_user', 'fake_proj_id', domain_id='default')
quota_utils._keystone_client(domain_context)
api_utils._keystone_client(domain_context)
ks_token.assert_called_once_with(
auth_url=self.auth_url, token=domain_context.auth_token,
domain_id=domain_context.domain_id)
@ -85,51 +79,11 @@ class QuotaUtilsTest(test.TestCase):
self, ks_token, ksclient_session, ksclient_class):
project_context = context.RequestContext(
'fake_user', project_id=fake.PROJECT_ID)
quota_utils._keystone_client(project_context)
api_utils._keystone_client(project_context)
ks_token.assert_called_once_with(
auth_url=self.auth_url, token=project_context.auth_token,
project_id=project_context.project_id)
@mock.patch('keystoneclient.client.Client')
def test_get_project_keystoneclient_v2(self, ksclient_class):
keystoneclient = ksclient_class.return_value
keystoneclient.version = 'v2.0'
expected_project = quota_utils.GenericProjectInfo(
self.context.project_id, 'v2.0')
project = quota_utils.get_project_hierarchy(
self.context, self.context.project_id)
self.assertEqual(expected_project.__dict__, project.__dict__)
@mock.patch('keystoneclient.client.Client')
def test_get_project_keystoneclient_v3(self, ksclient_class):
keystoneclient = ksclient_class.return_value
keystoneclient.version = 'v3'
returned_project = self.FakeProject(self.context.project_id, 'bar')
del returned_project.subtree
keystoneclient.projects.get.return_value = returned_project
expected_project = quota_utils.GenericProjectInfo(
self.context.project_id, 'v3', 'bar', domain_id='default')
project = quota_utils.get_project_hierarchy(
self.context, self.context.project_id)
self.assertEqual(expected_project.__dict__, project.__dict__)
@mock.patch('keystoneclient.client.Client')
def test_get_project_keystoneclient_v3_with_subtree(self, ksclient_class):
keystoneclient = ksclient_class.return_value
keystoneclient.version = 'v3'
returned_project = self.FakeProject(self.context.project_id, 'bar')
subtree_dict = {'baz': {'quux': None}}
returned_project.subtree = subtree_dict
keystoneclient.projects.get.return_value = returned_project
expected_project = quota_utils.GenericProjectInfo(
self.context.project_id, 'v3', 'bar', subtree_dict,
domain_id='default')
project = quota_utils.get_project_hierarchy(
self.context, self.context.project_id, subtree_as_ids=True)
keystoneclient.projects.get.assert_called_once_with(
self.context.project_id, parents_as_ids=False, subtree_as_ids=True)
self.assertEqual(expected_project.__dict__, project.__dict__)
def _setup_mock_ksclient(self, mock_client, version='v3',
subtree=None, parents=None):
keystoneclient = mock_client.return_value
@ -141,50 +95,6 @@ class QuotaUtilsTest(test.TestCase):
proj.parent_id = next(iter(parents.keys()))
keystoneclient.projects.get.return_value = proj
@mock.patch('keystoneclient.client.Client')
def test__filter_domain_id_from_parents_domain_as_parent(
self, mock_client):
# Test with a top level project (domain is direct parent)
self._setup_mock_ksclient(mock_client, parents={'default': None})
project = quota_utils.get_project_hierarchy(
self.context, self.context.project_id, parents_as_ids=True)
self.assertIsNone(project.parent_id)
self.assertIsNone(project.parents)
@mock.patch('keystoneclient.client.Client')
def test__filter_domain_id_from_parents_domain_as_grandparent(
self, mock_client):
# Test with a child project (domain is more than a parent)
self._setup_mock_ksclient(mock_client,
parents={'bar': {'default': None}})
project = quota_utils.get_project_hierarchy(
self.context, self.context.project_id, parents_as_ids=True)
self.assertEqual('bar', project.parent_id)
self.assertEqual({'bar': None}, project.parents)
@mock.patch('keystoneclient.client.Client')
def test__filter_domain_id_from_parents_no_domain_in_parents(
self, mock_client):
# Test that if top most parent is not a domain (to simulate an older
# keystone version) nothing gets removed from the tree
parents = {'bar': {'foo': None}}
self._setup_mock_ksclient(mock_client, parents=parents)
project = quota_utils.get_project_hierarchy(
self.context, self.context.project_id, parents_as_ids=True)
self.assertEqual('bar', project.parent_id)
self.assertEqual(parents, project.parents)
@mock.patch('keystoneclient.client.Client')
def test__filter_domain_id_from_parents_no_parents(
self, mock_client):
# Test that if top no parents are present (to simulate an older
# keystone version) things don't blow up
self._setup_mock_ksclient(mock_client)
project = quota_utils.get_project_hierarchy(
self.context, self.context.project_id, parents_as_ids=True)
self.assertIsNone(project.parent_id)
self.assertIsNone(project.parents)
def _process_reserve_over_quota(self, overs, usages, quotas,
expected_ex,
resource='volumes'):

38
cinder/tests/unit/test_utils.py
View File

@ -24,6 +24,7 @@ import webob.exc
import cinder
from cinder.api import api_utils
from cinder import context
from cinder import exception
from cinder.tests.unit import fake_constants as fake
from cinder.tests.unit import test
@ -1415,3 +1416,40 @@ class LimitOperationsTestCase(test.TestCase):
mock_exec.assert_called_once_with(mocked_semaphore.__enter__)
mocked_semaphore.__exit__.assert_not_called()
mocked_semaphore.__exit__.assert_called_once_with(None, None, None)
class TestKeystoneProjectGet(test.TestCase):
class FakeProject(object):
def __init__(self, id='foo', name=None):
self.id = id
self.name = name
self.description = 'fake project description'
self.domain_id = 'default'
@mock.patch('keystoneclient.client.Client')
def test_get_project_keystoneclient_v2(self, ksclient_class):
self.context = context.RequestContext('fake_user', 'fake_proj_id')
keystoneclient = ksclient_class.return_value
keystoneclient.version = 'v2.0'
returned_project = self.FakeProject(self.context.project_id, 'bar')
keystoneclient.projects.get.return_value = returned_project
expected_project = api_utils.GenericProjectInfo(
self.context.project_id, 'v2.0', domain_id='default', name='bar',
description='fake project description')
project = api_utils.get_project(
self.context, self.context.project_id)
self.assertEqual(expected_project.__dict__, project.__dict__)
@mock.patch('keystoneclient.client.Client')
def test_get_project_keystoneclient_v3(self, ksclient_class):
self.context = context.RequestContext('fake_user', 'fake_proj_id')
keystoneclient = ksclient_class.return_value
keystoneclient.version = 'v3'
returned_project = self.FakeProject(self.context.project_id, 'bar')
keystoneclient.projects.get.return_value = returned_project
expected_project = api_utils.GenericProjectInfo(
self.context.project_id, 'v3', domain_id='default', name='bar',
description='fake project description')
project = api_utils.get_project(
self.context, self.context.project_id)
self.assertEqual(expected_project.__dict__, project.__dict__)