openstack/glance
Code Issues Proposed changes

Fix DeprecationWarnings for RequestContext.tenant/user usage

Browse Source 
RequestContext.tenant and user fields are deprecated in favor
of project_id and user_id respectively.

This change modifies the glance.context.RequestContext constructor
to transition usage of tenant/user to project_id/user_id until
all tests are moved over to the new attributes. Runtime usage of
the old fiels is updated.

To prevent new code from using the deprecated fields, a warnings
filter is added which will make tests fail if they hit code using
the old fields.

Co-Authored-By: Abhishek Kekane <akekane@redhat.com>
Change-Id: I351380840308a24769ece93abc6d1a9a6d6aa06f
This commit is contained in:
Matt Riedemann 2019-02-06 10:30:47 +00:00
parent 1d3f9ed234
commit 88a8ad7823
14 changed files with 56 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
glance/api/common.py
View File

@ -76,8 +76,8 @@ def image_send_notification(bytes_written, expected_size, image_meta, request,
'bytes_sent': bytes_written,
'image_id': image_meta['id'],
'owner_id': image_meta['owner'],
'receiver_tenant_id': context.tenant,
'receiver_user_id': context.user,
'receiver_tenant_id': context.project_id,
'receiver_user_id': context.user_id,
'destination_ip': request.remote_addr,
}
if bytes_written != expected_size:
@ -156,7 +156,7 @@ def check_quota(context, image_size, db_api, image_id=None):
if remaining is None:
return
user = getattr(context, 'user', '<unknown>')
user = getattr(context, 'user_id', '<unknown>')
if image_size is None:
# NOTE(jbresnah) When the image size is None it means that it is

2
glance/api/v2/image_data.py
View File

@ -133,7 +133,7 @@ class ImageDataController(object):
'keystone.token_info')['token']['roles']:
roles.append(role_info['name'])
refresher = trust_auth.TokenRefresher(user_plugin,
cxt.tenant,
cxt.project_id,
roles)
except Exception as e:
LOG.info(_LI("Unable to create trust: %s "

13
glance/context.py
View File

@ -28,6 +28,15 @@ class RequestContext(context.RequestContext):
def __init__(self, owner_is_tenant=True, service_catalog=None,
policy_enforcer=None, **kwargs):
# TODO(mriedem): Remove usage of user and tenant from old tests.
if 'tenant' in kwargs:
# Prefer project_id if passed, otherwise alias tenant as project_id
tenant = kwargs.pop('tenant')
kwargs['project_id'] = kwargs.get('project_id', tenant)
if 'user' in kwargs:
# Prefer user_id if passed, otherwise alias user as user_id
user = kwargs.pop('user')
kwargs['user_id'] = kwargs.get('user_id', user)
super(RequestContext, self).__init__(**kwargs)
self.owner_is_tenant = owner_is_tenant
self.service_catalog = service_catalog
@ -45,8 +54,8 @@ class RequestContext(context.RequestContext):
def to_policy_values(self):
pdict = super(RequestContext, self).to_policy_values()
pdict['user'] = self.user_id
pdict['tenant'] = self.project_id
pdict['user_id'] = self.user_id
pdict['project_id'] = self.project_id
return pdict
@classmethod

4
glance/notifier.py
View File

@ -394,8 +394,8 @@ class ImageProxy(NotificationProxy, domain_proxy.Image):
'bytes_sent': bytes_sent,
'image_id': self.repo.image_id,
'owner_id': self.repo.owner,
'receiver_tenant_id': self.context.tenant,
'receiver_user_id': self.context.user,
'receiver_tenant_id': self.context.project_id,
'receiver_user_id': self.context.user_id,
}
def _get_chunk_data_iterator(self, data, chunk_size=None):

4
glance/registry/client/v1/api.py
View File

@ -153,8 +153,8 @@ def get_registry_client(cxt):
if CONF.send_identity_headers:
identity_headers = {
'X-User-Id': cxt.user or '',
'X-Tenant-Id': cxt.tenant or '',
'X-User-Id': cxt.user_id or '',
'X-Tenant-Id': cxt.project_id or '',
'X-Roles': ','.join(cxt.roles),
'X-Identity-Status': 'Confirmed',
'X-Service-Catalog': jsonutils.dumps(cxt.service_catalog),

10
glance/tests/functional/db/base.py
View File

@ -1580,7 +1580,7 @@ class TaskTests(test_utils.BaseTestCase):
db_tests.reset_db(self.db_api)
def build_task_fixtures(self):
self.context.tenant = str(uuid.uuid4())
self.context.project_id = str(uuid.uuid4())
fixtures = [
{
'owner': self.context.owner,
@ -1816,7 +1816,7 @@ class TaskTests(test_utils.BaseTestCase):
def test_task_create(self):
task_id = str(uuid.uuid4())
self.context.tenant = self.context.owner
self.context.project_id = self.context.owner
values = {
'id': task_id,
'owner': self.context.owner,
@ -1834,7 +1834,7 @@ class TaskTests(test_utils.BaseTestCase):
def test_task_create_with_all_task_info_null(self):
task_id = str(uuid.uuid4())
self.context.tenant = str(uuid.uuid4())
self.context.project_id = str(uuid.uuid4())
values = {
'id': task_id,
'owner': self.context.owner,
@ -1856,7 +1856,7 @@ class TaskTests(test_utils.BaseTestCase):
self.assertIsNone(task['message'])
def test_task_update(self):
self.context.tenant = str(uuid.uuid4())
self.context.project_id = str(uuid.uuid4())
result = {'foo': 'bar'}
task_values = build_task_fixture(owner=self.context.owner,
result=result)
@ -1883,7 +1883,7 @@ class TaskTests(test_utils.BaseTestCase):
self.assertGreater(task['updated_at'], task['created_at'])
def test_task_update_with_all_task_info_null(self):
self.context.tenant = str(uuid.uuid4())
self.context.project_id = str(uuid.uuid4())
task_values = build_task_fixture(owner=self.context.owner,
input=None,
result=None,

8
glance/tests/functional/v2/registry_data_api.py
View File

@ -29,10 +29,10 @@ def patched_bulk_request(self, commands):
# present)
body = self._serializer.to_json(commands)
headers = {"X-Identity-Status": "Confirmed", 'X-Roles': 'member'}
if self.context.user is not None:
headers['X-User-Id'] = self.context.user
if self.context.tenant is not None:
headers['X-Tenant-Id'] = self.context.tenant
if self.context.user_id is not None:
headers['X-User-Id'] = self.context.user_id
if self.context.project_id is not None:
headers['X-Tenant-Id'] = self.context.project_id
response = super(RPCClient, self).do_request('POST',
self.base_path,
body,

18
glance/tests/functional/v2/test_images.py
View File

@ -1653,7 +1653,7 @@ class TestImages(functional.FunctionalTest):
"default": "",
"add_image": "",
"get_image": "",
"modify_image": "tenant:%(owner)s",
"modify_image": "project_id:%(owner)s",
"upload_image": "",
"get_image_location": "",
"delete_image": "",
@ -1744,7 +1744,7 @@ class TestImages(functional.FunctionalTest):
"restricted":
"not ('aki':%(container_format)s and role:_member_)",
"download_image": "role:admin or rule:restricted",
"add_member": "tenant:%(owner)s",
"add_member": "project_id:%(owner)s",
}
self.set_policy_rules(rules)
@ -1809,7 +1809,7 @@ class TestImages(functional.FunctionalTest):
"context_is_admin": "role:admin",
"default": "",
"add_image": "",
"get_image": "tenant:%(owner)s",
"get_image": "project_id:%(owner)s",
"modify_image": "",
"upload_image": "",
"get_image_location": "",
@ -1858,8 +1858,8 @@ class TestImages(functional.FunctionalTest):
"context_is_admin": "role:admin",
"default": "",
"add_image": "",
"publicize_image": "tenant:%(owner)s",
"get_image": "tenant:%(owner)s",
"publicize_image": "project_id:%(owner)s",
"get_image": "project_id:%(owner)s",
"modify_image": "",
"upload_image": "",
"get_image_location": "",
@ -1900,8 +1900,8 @@ class TestImages(functional.FunctionalTest):
"context_is_admin": "role:admin",
"default": "",
"add_image": "",
"communitize_image": "tenant:%(owner)s",
"get_image": "tenant:%(owner)s",
"communitize_image": "project_id:%(owner)s",
"get_image": "project_id:%(owner)s",
"modify_image": "",
"upload_image": "",
"get_image_location": "",
@ -1942,8 +1942,8 @@ class TestImages(functional.FunctionalTest):
"context_is_admin": "role:admin",
"default": "",
"add_image": "",
"publicize_image": "tenant:%(owner)s",
"get_image": "tenant:%(owner)s",
"publicize_image": "project_id:%(owner)s",
"get_image": "project_id:%(owner)s",
"modify_image": "",
"upload_image": "",
"get_image_location": "",

5
glance/tests/unit/fixtures.py
View File

@ -131,4 +131,9 @@ class WarningsFixture(pyfixtures.Fixture):
' This key is deprecated. Please update your policy '
'file to use the standard policy values.')
# NOTE(mriedem): user/tenant is deprecated in oslo.context so don't
# let anything new use it
warnings.filterwarnings(
'error', message="Property '.*' has moved to '.*'")
self.addCleanup(warnings.resetwarnings)

12
glance/tests/unit/test_context_middleware.py
View File

@ -41,8 +41,8 @@ class TestContextMiddleware(base.IsolatedUnitTest):
req = self._build_request()
self._build_middleware().process_request(req)
self.assertEqual('token1', req.context.auth_token)
self.assertEqual('user1', req.context.user)
self.assertEqual('tenant1', req.context.tenant)
self.assertEqual('user1', req.context.user_id)
self.assertEqual('tenant1', req.context.project_id)
self.assertEqual(['role1', 'role2'], req.context.roles)
def test_is_admin_flag(self):
@ -93,8 +93,8 @@ class TestContextMiddleware(base.IsolatedUnitTest):
middleware = self._build_middleware()
middleware.process_request(req)
self.assertIsNone(req.context.auth_token)
self.assertIsNone(req.context.user)
self.assertIsNone(req.context.tenant)
self.assertIsNone(req.context.user_id)
self.assertIsNone(req.context.project_id)
self.assertEqual([], req.context.roles)
self.assertFalse(req.context.is_admin)
self.assertTrue(req.context.read_only)
@ -141,8 +141,8 @@ class TestUnauthenticatedContextMiddleware(base.IsolatedUnitTest):
req = webob.Request.blank('/')
middleware.process_request(req)
self.assertIsNone(req.context.auth_token)
self.assertIsNone(req.context.user)
self.assertIsNone(req.context.tenant)
self.assertIsNone(req.context.user_id)
self.assertIsNone(req.context.project_id)
self.assertEqual([], req.context.roles)
self.assertTrue(req.context.is_admin)

8
glance/tests/unit/utils.py
View File

@ -230,9 +230,9 @@ class FakeStoreAPI(object):
size = len(data.data.fd)
if (current_store_size + size) > store_max_size:
raise exception.StorageFull()
if context.user == USER2:
if context.user_id == USER2:
raise exception.Forbidden()
if context.user == USER3:
if context.user_id == USER3:
raise exception.StorageWriteDenied()
self.data[image_id] = (data, size)
checksum = 'Z'
@ -253,9 +253,9 @@ class FakeStoreAPI(object):
size = len(data.data.fd)
if (current_store_size + size) > store_max_size:
raise exception.StorageFull()
if context.user == USER2:
if context.user_id == USER2:
raise exception.Forbidden()
if context.user == USER3:
if context.user_id == USER3:
raise exception.StorageWriteDenied()
self.data[image_id] = (data, size)
checksum = 'Z'

2
glance/tests/unit/v2/test_image_data_resource.py
View File

@ -403,7 +403,7 @@ class TestImagesController(base.StoreClearingUnitTest):
# check that token has been correctly acquired and deleted
mock_refresher.assert_called_once_with(
request.environ['keystone.token_auth'],
request.context.tenant, ['FakeRole'])
request.context.project_id, ['FakeRole'])
refresher.refresh_token.assert_called_once_with()
refresher.release_resources.assert_called_once_with()
self.assertEqual("fake_token", request.context.auth_token)

4
glance/tests/unit/v2/test_images_resource.py
View File

@ -651,7 +651,7 @@ class TestImagesController(base.IsolatedUnitTest):
def test_show_not_allowed(self):
request = unit_test_utils.get_fake_request()
self.assertEqual(TENANT1, request.context.tenant)
self.assertEqual(TENANT1, request.context.project_id)
self.assertRaises(webob.exc.HTTPNotFound,
self.controller.show, request, UUID4)
@ -2669,7 +2669,7 @@ class TestImagesController(base.IsolatedUnitTest):
request = unit_test_utils.get_fake_request()
# NOTE(abhishekk): For coverage purpose setting tenant to
# None. It is not expected to do in normal scenarios.
request.context.tenant = None
request.context.project_id = None
with mock.patch.object(
glance.api.authorization.ImageRepoProxy, 'get') as mock_get:
mock_get.return_value = FakeImage(status='uploading')

2
glance/tests/unit/v2/test_tasks_resource.py
View File

@ -288,7 +288,7 @@ class TestTasksController(test_utils.BaseTestCase):
def test_get_not_allowed(self):
request = unit_test_utils.get_fake_request()
self.assertEqual(TENANT1, request.context.tenant)
self.assertEqual(TENANT1, request.context.project_id)
self.assertRaises(webob.exc.HTTPNotFound,
self.controller.get, request, UUID4)